On 2020/4/17 ??4:46, Michael S. Tsirkin wrote:> On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: >> On 2020/4/17 ??4:29, Michael S. Tsirkin wrote: >>> On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: >>>> On 2020/4/17 ??2:33, Michael S. Tsirkin wrote: >>>>> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: >>>>>> On 2020/4/17 ??6:55, Michael S. Tsirkin wrote: >>>>>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >>>>>>>> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >>>>>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >>>>>>>> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >>>>>>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >>>>>>>> without the caring of CONFIG_VHOST. >>>>>>>> >>>>>>>> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >>>>>>>> for the ones that doesn't want vhost. So it actually shifts the >>>>>>>> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >>>>>>>> not set". So this patch tries to enable CONFIG_VHOST explicitly in >>>>>>>> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >>>>>>>> >>>>>>>> Acked-by: Christian Borntraeger<borntraeger at de.ibm.com> (s390) >>>>>>>> Acked-by: Michael Ellerman<mpe at ellerman.id.au> (powerpc) >>>>>>>> Cc: Thomas Bogendoerfer<tsbogend at alpha.franken.de> >>>>>>>> Cc: Benjamin Herrenschmidt<benh at kernel.crashing.org> >>>>>>>> Cc: Paul Mackerras<paulus at samba.org> >>>>>>>> Cc: Michael Ellerman<mpe at ellerman.id.au> >>>>>>>> Cc: Heiko Carstens<heiko.carstens at de.ibm.com> >>>>>>>> Cc: Vasily Gorbik<gor at linux.ibm.com> >>>>>>>> Cc: Christian Borntraeger<borntraeger at de.ibm.com> >>>>>>>> Reported-by: Geert Uytterhoeven<geert at linux-m68k.org> >>>>>>>> Signed-off-by: Jason Wang<jasowang at redhat.com> >>>>>>> I rebased this on top of OABI fix since that >>>>>>> seems more orgent to fix. >>>>>>> Pushed to my vhost branch pls take a look and >>>>>>> if possible test. >>>>>>> Thanks! >>>>>> I test this patch by generating the defconfigs that wants vhost_net or >>>>>> vhost_vsock. All looks fine. >>>>>> >>>>>> But having CONFIG_VHOST_DPN=y may end up with the similar situation that >>>>>> this patch want to address. >>>>>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another >>>>>> menuconfig for VHOST_RING and do something similar? >>>>>> >>>>>> Thanks >>>>> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just >>>>> an internal variable for the OABI fix. I kept it separate >>>>> so it's easy to revert for 5.8. Yes we could squash it into >>>>> VHOST directly but I don't see how that changes logic at all. >>>> Sorry for being unclear. >>>> >>>> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left >>>> in the defconfigs. >>> But who cares? >> FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > The complaint was not about the symbol IIUC. It was that we caused > everyone to build vhost unless they manually disabled it.There could be some misunderstanding here. I thought it's somehow similar: a CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is not set. Thanks>
Michael S. Tsirkin
2020-Apr-17 08:57 UTC
[PATCH V2] vhost: do not enable VHOST_MENU by default
On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote:> > On 2020/4/17 ??4:46, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > On 2020/4/17 ??4:29, Michael S. Tsirkin wrote: > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > On 2020/4/17 ??2:33, Michael S. Tsirkin wrote: > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > On 2020/4/17 ??6:55, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger at de.ibm.com> (s390) > > > > > > > > > Acked-by: Michael Ellerman<mpe at ellerman.id.au> (powerpc) > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend at alpha.franken.de> > > > > > > > > > Cc: Benjamin Herrenschmidt<benh at kernel.crashing.org> > > > > > > > > > Cc: Paul Mackerras<paulus at samba.org> > > > > > > > > > Cc: Michael Ellerman<mpe at ellerman.id.au> > > > > > > > > > Cc: Heiko Carstens<heiko.carstens at de.ibm.com> > > > > > > > > > Cc: Vasily Gorbik<gor at linux.ibm.com> > > > > > > > > > Cc: Christian Borntraeger<borntraeger at de.ibm.com> > > > > > > > > > Reported-by: Geert Uytterhoeven<geert at linux-m68k.org> > > > > > > > > > Signed-off-by: Jason Wang<jasowang at redhat.com> > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > seems more orgent to fix. > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > if possible test. > > > > > > > > Thanks! > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > this patch want to address. > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > Thanks > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > Sorry for being unclear. > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > in the defconfigs. > > > > But who cares? > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > The complaint was not about the symbol IIUC. It was that we caused > > everyone to build vhost unless they manually disabled it. > > > There could be some misunderstanding here. I thought it's somehow similar: a > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > not set. > > ThanksHmm. So looking at Documentation/kbuild/kconfig-language.rst : Things that merit "default y/m" include: a) A new Kconfig option for something that used to always be built should be "default y". b) A new gatekeeping Kconfig option that hides/shows other Kconfig options (but does not generate any code of its own), should be "default y" so people will see those other options. c) Sub-driver behavior or similar options for a driver that is "default n". This allows you to provide sane defaults. So it looks like VHOST_MENU is actually matching rule b). So what's the problem we are trying to solve with this patch, exactly? Geert could you clarify pls?> > >
Michael S. Tsirkin
2020-Apr-17 09:01 UTC
[PATCH V2] vhost: do not enable VHOST_MENU by default
On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote:> > On 2020/4/17 ??4:46, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > On 2020/4/17 ??4:29, Michael S. Tsirkin wrote: > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > On 2020/4/17 ??2:33, Michael S. Tsirkin wrote: > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > On 2020/4/17 ??6:55, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger at de.ibm.com> (s390) > > > > > > > > > Acked-by: Michael Ellerman<mpe at ellerman.id.au> (powerpc) > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend at alpha.franken.de> > > > > > > > > > Cc: Benjamin Herrenschmidt<benh at kernel.crashing.org> > > > > > > > > > Cc: Paul Mackerras<paulus at samba.org> > > > > > > > > > Cc: Michael Ellerman<mpe at ellerman.id.au> > > > > > > > > > Cc: Heiko Carstens<heiko.carstens at de.ibm.com> > > > > > > > > > Cc: Vasily Gorbik<gor at linux.ibm.com> > > > > > > > > > Cc: Christian Borntraeger<borntraeger at de.ibm.com> > > > > > > > > > Reported-by: Geert Uytterhoeven<geert at linux-m68k.org> > > > > > > > > > Signed-off-by: Jason Wang<jasowang at redhat.com> > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > seems more orgent to fix. > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > if possible test. > > > > > > > > Thanks! > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > this patch want to address. > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > Thanks > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > Sorry for being unclear. > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > in the defconfigs. > > > > But who cares? > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > The complaint was not about the symbol IIUC. It was that we caused > > everyone to build vhost unless they manually disabled it. > > > There could be some misunderstanding here. I thought it's somehow similar: a > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > not set. > > Thanks >BTW do entries with no prompt actually appear in defconfig?> >
Geert Uytterhoeven
2020-Apr-17 09:25 UTC
[PATCH V2] vhost: do not enable VHOST_MENU by default
Hi Michael, On Fri, Apr 17, 2020 at 10:57 AM Michael S. Tsirkin <mst at redhat.com> wrote:> On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote: > > On 2020/4/17 ??4:46, Michael S. Tsirkin wrote: > > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > > On 2020/4/17 ??4:29, Michael S. Tsirkin wrote: > > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > > On 2020/4/17 ??2:33, Michael S. Tsirkin wrote: > > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > > On 2020/4/17 ??6:55, Michael S. Tsirkin wrote: > > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger at de.ibm.com> (s390) > > > > > > > > > > Acked-by: Michael Ellerman<mpe at ellerman.id.au> (powerpc) > > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend at alpha.franken.de> > > > > > > > > > > Cc: Benjamin Herrenschmidt<benh at kernel.crashing.org> > > > > > > > > > > Cc: Paul Mackerras<paulus at samba.org> > > > > > > > > > > Cc: Michael Ellerman<mpe at ellerman.id.au> > > > > > > > > > > Cc: Heiko Carstens<heiko.carstens at de.ibm.com> > > > > > > > > > > Cc: Vasily Gorbik<gor at linux.ibm.com> > > > > > > > > > > Cc: Christian Borntraeger<borntraeger at de.ibm.com> > > > > > > > > > > Reported-by: Geert Uytterhoeven<geert at linux-m68k.org> > > > > > > > > > > Signed-off-by: Jason Wang<jasowang at redhat.com> > > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > > seems more orgent to fix. > > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > > if possible test. > > > > > > > > > Thanks! > > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > > this patch want to address. > > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > > > Thanks > > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > > Sorry for being unclear. > > > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > > in the defconfigs. > > > > > But who cares? > > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > > The complaint was not about the symbol IIUC. It was that we caused > > > everyone to build vhost unless they manually disabled it. > > > > There could be some misunderstanding here. I thought it's somehow similar: a > > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > > not set. > > > > Thanks > > Hmm. So looking at Documentation/kbuild/kconfig-language.rst : > > Things that merit "default y/m" include: > > a) A new Kconfig option for something that used to always be built > should be "default y". > > b) A new gatekeeping Kconfig option that hides/shows other Kconfig > options (but does not generate any code of its own), should be > "default y" so people will see those other options. > > c) Sub-driver behavior or similar options for a driver that is > "default n". This allows you to provide sane defaults. > > > So it looks like VHOST_MENU is actually matching rule b). > So what's the problem we are trying to solve with this patch, exactly? > > Geert could you clarify pls?I can confirm VHOST_MENU is matching rule b), so it is safe to always enable it. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 2020/4/17 ??5:01, Michael S. Tsirkin wrote:>> There could be some misunderstanding here. I thought it's somehow similar: a >> CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is >> not set. >> >> Thanks >> > BTW do entries with no prompt actually appear in defconfig? >Yes. I can see CONFIG_VHOST_DPN=y after make ARCH=m68k defconfig Thanks