Messages in this thread | | | Date | Wed, 27 Apr 2022 18:21:51 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH v2] export: fix string handling of namespace in EXPORT_SYMBOL_NS |
| |
On Thu, Apr 28, 2022 at 12:11:13AM +0900, Masahiro Yamada wrote: > On Wed, Apr 27, 2022 at 11:50 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Apr 27, 2022 at 11:29:19PM +0900, Masahiro Yamada wrote: > > > On Wed, Apr 27, 2022 at 6:06 PM Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > Commit c3a6cf19e695 ("export: avoid code duplication in > > > > include/linux/export.h") broke the ability for a defined string to be > > > > used as a namespace value. > > > > > > In hindsight, this was a bad idea. > > > > > > > > > EXPORT_SYMBOL_NS_GPL(dw_spi_resume_host, "SPI_DW_CORE") > > > > > > is much much better than: > > > > > > EXPORT_SYMBOL_NS_GPL(dw_spi_resume_host, SPI_DW_CORE) > > > > I agree, but it's really not that big of a deal. We could change it if > > you want. > > > > > ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=USB_STORAGE > > > > > > is also a bad idea. > > > > That's not such a bad idea as it lets you set a namespace for a > > directory and below easily. What would you want to use instead? > > I like "explicit" better than "implicit". > > With EXPORT_SYMBOL_GPL(usb_stor_resume); > you need to check Makefile to know whether it is global > or is in a specific namespace. > > EXPORT_SYMBOL_NS_GPL(usb_stor_resume, "USB_STORAGE"); > will clarify the namespace at a glance.
I agree, but currently I use the makefile change in a big out-of-tree project to do "all exported symbols in this directory and lower are now in this namespace". That's a huge help and makes for a 2 line change vs. a hundreds-of-lines change.
> > > When you look at EXPORT_SYMBOL_GPL() in C files, you will not be > > > aware of the presence of the namespace. > > > > It's easy to tell when things do not link properly :) > > Right, modpost will tell you that once you compile it.
And that's the best thing, 'make nsdeps' will then even generate a patch to fix things up.
> > > Anyway, it is presumably too late to fix it. > > > > Not really, the number of in-kernel users are still small and can be > > changed if you like. External users can update when they hit the change > > as well, not a big deal. > > > > Other than using a string for the namespace definition, what would you > > like to see done differently? > > Nothing else. > > (but I am not a big fan of the module namespace itself.)
I think more subsystems need to start using them as it instantly shows where some drivers are doing things that maybe they shouldn't be doing. It was insightful when the dma-buf code moved to a module namespace as it found places the maintainers were not even aware of.
Anyway, it's on my long-term todo list...
thanks,
greg k-h
| |