Re: [PATCH kvmtool] Fixes: 0febaae00bb6 ("Add asm/kernel.h for riscv")

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, May 23, 2022 at 12:15:55PM -0700, Dao Lu wrote:
> Hi Alex,
> 
> After talking with my colleague I have some additional questions about
> what number we want to put there, as of now there is already a patch
> that will increase the range in Kconfig to 2-512:
> 
> https://lore.kernel.org/lkml/CAOnJCUJrN4frY_OdQzO-yr5CrDLvj=ge9KY2d=XnGvAF-uQNvQ@xxxxxxxxxxxxxx/T/
> 
> It seems like a moving target and as riscv develops we kinda expect
> this number will grow further. Do you think it is ok for me to at
> least set it to 512, if not 4096 at this time?

It's up to you in the end, I'm not familiar with the riscv architecture.

NR_CPUS is only used at the moment when creating a cpumask, where it
represents the maximum number of bits in the bitmask (bits, not bytes). So
NR_CPUS=512 means that the cpumask will contain 8 unsigned longs, while with
4096 it will contain 64 unsigned longs.

Cpumasks are only used by the arm64 code so far, so the value that you
choose won't affect existing code.

Thanks,
Alex

> 
> Thanks,
> Dao
> 
> On Mon, May 23, 2022 at 9:27 AM Alexandru Elisei
> <alexandru.elisei@xxxxxxx> wrote:
> >
> > Hi,
> >
> > On Mon, May 23, 2022 at 09:04:04AM -0700, Dao Lu wrote:
> > > Hi Alex,
> > >
> > > Thanks for pointing that out - I wasn't sure where the number came
> > > from so I basically copied from the arm one just so the compilation
> > > can pass.
> >
> > I see, I was worried that I was looking in the wrong place.
> >
> > >
> > > I am happy to fix up the number to 32 and add the compile error
> > > message to the commit message like you said - would something like
> > > this work?
> > > -------
> > > Fixes the following compilation issue:
> > >
> > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file
> > > or directory
> > >     5 | #include "asm/kernel.h"
> > > -------
> >
> > Sounds good, thanks:
> > Tested-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> >
> > With the error message added:
> > Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> >
> > Thanks,
> > Alex
> >
> > > Thanks,
> > > Dao
> > >
> > > On Mon, May 23, 2022 at 2:33 AM Alexandru Elisei
> > > <alexandru.elisei@xxxxxxx> wrote:
> > > >
> > > > Adding the kvmtool maintainers, I just noticed that they were missing.
> > > >
> > > > On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> > > > > Hi,
> > > > >
> > > > > When I started working on the heterogeneous PMU series, support for the
> > > > > riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> > > > > missed adding the header file.
> > > > >
> > > > > This indeed fixes this compilation error:
> > > > >
> > > > > In file included from include/linux/rbtree.h:32,
> > > > >                  from include/kvm/devices.h:4,
> > > > >                  from include/kvm/pci.h:10,
> > > > >                  from include/kvm/vfio.h:6,
> > > > >                  from include/kvm/kvm-config.h:5,
> > > > >                  from include/kvm/kvm.h:6:
> > > > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
> > > > >     5 | #include "asm/kernel.h"
> > > > >       |          ^~~~~~~~~~~~~~
> > > > > cc1: all warnings being treated as errors
> > > > > compilation terminated.
> > > > > make: *** [Makefile:484: builtin-balloon.o] Error 1
> > > > >
> > > > > Would be nice to include it in the commit message, so people googling for
> > > > > that exact error message can come across this commit.
> > > > >
> > > > > On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > > > > > Signed-off-by: Dao Lu <daolu@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >  riscv/include/asm/kernel.h | 8 ++++++++
> > > > > >  1 file changed, 8 insertions(+)
> > > > > >  create mode 100644 riscv/include/asm/kernel.h
> > > > > >
> > > > > > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > > > > > new file mode 100644
> > > > > > index 0000000..a2a8d9e
> > > > > > --- /dev/null
> > > > > > +++ b/riscv/include/asm/kernel.h
> > > > > > @@ -0,0 +1,8 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > > +
> > > > > > +#ifndef __ASM_KERNEL_H
> > > > > > +#define __ASM_KERNEL_H
> > > > > > +
> > > > > > +#define NR_CPUS    4096
> > > > >
> > > > > In arch/riscv/Kconfig I see this:
> > > > >
> > > > > config NR_CPUS
> > > > >       int "Maximum number of CPUs (2-32)"
> > > > >       range 2 32
> > > > >       depends on SMP
> > > > >       default "8"
> > > > >
> > > > > Would you mind explaining where the 4096 number of CPUs comes from?
> > > > >
> > > > > Thanks,
> > > > > Alex
> > > > >
> > > > > > +
> > > > > > +#endif /* __ASM_KERNEL_H */
> > > > > > --
> > > > > > 2.36.0
> > > > > >



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux