linux-kernel - Re: [PATCH] staging: vme: Adjusted VME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YkaXRpIElW1BwKGb@kroah.com>
Date:   Fri, 1 Apr 2022 08:10:14 +0200
From:   reg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Bruno Moreira-Guedes <codeagain@...eagain.dev>
Cc:     Martyn Welch <martyn@...chs.me.uk>,
        Manohar Vanga <manohar.vanga@...il.com>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        outreachy@...ts.linux.dev
Subject: Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig

On Fri, Apr 01, 2022 at 08:08:17AM +0200, reg Kroah-Hartman wrote:
> On Fri, Apr 01, 2022 at 02:00:45AM -0300, Bruno Moreira-Guedes wrote:
> > Currently, the VME_USER driver is in the staging tree Kconfig, unlike
> > other VME drivers already moved to the main portions of the kernel tree.
> > Its configuration is, however, nested into the VME_BUS config option,
> > which might be misleading.
> > 
> > Since the staging tree "[...] is used to hold stand-alone[1] drivers and
> > filesystem that are not ready to be merged into the main portion of the
> > Linux kernel tree [...]"[1], IMHO all staging drivers should appear
> > nested into the Main Menu -> Device Drivers -> Staging Drivers to make
> > sure the user don't pick it without being fully aware of its staging
> > status as it could be the case in Menu -> Device Drivers -> VME bridge
> > support (the current location).
> > 
> > With this change menuconfig users will clearly know this is not a driver
> > in the main portion of the kernel tree and decide whether to build it or
> > not with that clearly in mind.
> > 
> > This change goes into the same direction of commit 4b4cdf3979c3
> > ("STAGING: Move staging drivers back to staging-specific menu")
> > 
> > [1] https://lkml.org/lkml/2009/3/18/314
> > 
> > Signed-off-by: Bruno Moreira-Guedes <codeagain@...eagain.dev>
> > ---
> >  drivers/staging/Kconfig | 2 ++
> >  drivers/vme/Kconfig     | 2 --
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> > index 932acb4e8cbc..0545850eb2ff 100644
> > --- a/drivers/staging/Kconfig
> > +++ b/drivers/staging/Kconfig
> > @@ -88,4 +88,6 @@ source "drivers/staging/qlge/Kconfig"
> >  
> >  source "drivers/staging/wfx/Kconfig"
> >  
> > +source "drivers/staging/vme/devices/Kconfig"
> > +
> >  endif # STAGING
> > diff --git a/drivers/vme/Kconfig b/drivers/vme/Kconfig
> > index 936392ca3c8c..c13dd9d2a604 100644
> > --- a/drivers/vme/Kconfig
> > +++ b/drivers/vme/Kconfig
> > @@ -15,6 +15,4 @@ source "drivers/vme/bridges/Kconfig"
> >  
> >  source "drivers/vme/boards/Kconfig"
> >  
> > -source "drivers/staging/vme/devices/Kconfig"
> > -
> >  endif # VME
> > -- 
> > 2.35.1
> > 
> > 
> 
> The problem with this change is that you just changed the initialization
> order of the drivers if they are built into the kernel.  Are you sure
> that you can initialize a vme device driver before the vme bridge and
> bus code is run?  I don't know if that will work properly, which is why
> the Kconfig entries are in the order they currently are in (we preserved
> the link order.)
> 
> It's not an obvious thing at all, sorry, but build order defines link
> order, which defines the order in which things are initialized in the
> kernel.
> 
> So I can't take this change unless you are able to prove that it still
> works properly on the hardware that these drivers control.  Do you have
> this hardware to test this change with?

Oh wait, it's the Makefile order that controls this, not the Kconfig
order.  Sorry for the noise here, it's still early...

So this change _should_ be fine, but it would be good if you could prove
it still works with some build tests.  How did you test this change?

thanks,

greg k-h