Re: [PATCH v3 15/34] xen/riscv: introduce atomic.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2024-01-23 at 14:30 +0100, Jan Beulich wrote:
> On 23.01.2024 13:24, Oleksii wrote:
> > On Tue, 2024-01-23 at 11:30 +0100, Jan Beulich wrote:
> > > On 23.01.2024 11:21, Oleksii wrote:
> > > > On Mon, 2024-01-22 at 17:56 +0100, Jan Beulich wrote:
> > > > > On 22.12.2023 16:12, Oleksii Kurochko wrote:
> > > > > > --- /dev/null
> > > > > > +++ b/xen/arch/riscv/include/asm/fence.h
> > > > > > @@ -0,0 +1,13 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > > > +#ifndef _ASM_RISCV_FENCE_H
> > > > > > +#define _ASM_RISCV_FENCE_H
> > > > > > +
> > > > > > +#ifdef CONFIG_SMP
> > > > > > +#define RISCV_ACQUIRE_BARRIER              "\tfence r , rw\n"
> > > > > > +#define RISCV_RELEASE_BARRIER              "\tfence rw,  w\n"
> > > > > > +#else
> > > > > > +#define RISCV_ACQUIRE_BARRIER
> > > > > > +#define RISCV_RELEASE_BARRIER
> > > > > > +#endif
> > > > > 
> > > > > Do you really care about the !SMP case? On x86 at least we
> > > > > stopped
> > > > > special-
> > > > > casing that configuration many years ago (the few cases where
> > > > > for
> > > > > typically
> > > > > build reasons it matters, using CONFIG_NR_CPUS is
> > > > > sufficient). If
> > > > > you
> > > > > care
> > > > > about it, there needs to be somewhere you actually #define
> > > > > CONFIG_SMP.
> > > > Can't we use instead of CONFIG_SMP - CONFIG_NR_CPUS?
> > > 
> > > You can. Question is whether there's a point in doing so. Do you
> > > expect people to actually want to run Xen on single-CPU systems?
> > > They're generally not overly well suited for virtualization ...
> > Just to clarify.
> > 
> > Do you mean physically single based CPU?
> > Then I don't expect to run Xen on such systems and it is not
> > nesessary
> > to define *_BARRIER in this case. Should we have to add build error
> > notification that we don't support single-CPU systems in this
> > header?
> > 
> > If you are speaking about we have ,let it be, 4 CPUs and only 1 CPU
> > is
> > currently supported by Xen then it still makes sense.
> 
> No, that's still not what I mean. The question is: Is it useful for
> you
> to _special case_ the NR_CPUS=1 case? Or is it instead simpler to
> handle
> NR_CPUS=1 the same as NR_CPUS>1 (accepting less than ideal
> performance,
> on the basis that in reality nobody's expected to use such in
> production
> anyway)?
NR_CPUS=1 sometimes is useful for debugging. At least, at the start I
used that several times, but ITBO I don't remember when I used that
case after SMP support was added and context_switch() was fixed.

Probably, I misunderstand the real idea of NR_CPUS. Does NR_CPUS
represent a number of logical CPUs which can be different from physical
amount of CPU?
If yes, then what I wrote above it was about physical CPU and then in
context of logical CPUs I don't need a special case when NR_CPUS=1.

~ Oleksii