Hello Andy, On Wed, Dec 04, 2024 at 12:28:23AM +0200, Andy Shevchenko wrote: > On Tue, Dec 03, 2024 at 10:32:17PM +0100, Uwe Kleine-König wrote: > > On Tue, Dec 03, 2024 at 09:12:36PM +0200, Andy Shevchenko wrote: > > > On Tue, Dec 03, 2024 at 06:16:14PM +0100, Uwe Kleine-König wrote: > > > > Each user of the exported symbols related to the pwm-lpss driver needs > > > > to import the matching namespace. So this can just be done in the header > > > > together with the prototypes. > > > > > > > > This fixes drivers/pinctrl/intel/pinctrl-intel.c which failed to import > > > > that namespace before. (However this didn't hurt because the pwm-lpss > > > > module namespace isn't used; see the next commit.) > > > > > > I disagree on this change, I think it had been discussed already. > > > > Who discussed this? If I was involved, I don't remember. So if you have > > a link, that would be great. > > Sure, https://lore.kernel.org/linux-pwm/20220908135658.64463-1-andriy.shevchenko@xxxxxxxxxxxxxxx/ > you were a participant there. Ah that. When I read "it had been discussed already" I expected a discussion with an agreement in the end. Let me note that you chose the more complicated way back then (and I let you) and the result is that the pinctrl driver didn't get the (up to now not yet) needed import. > > > The header must not provide any module importing features as it effectively > > > diminishes the point of namespace. Any (ab)user can include just a header and > > > be okay with that. > > > > Huh? Any abuser can also just do the IMPORT_NS statement? Module > > namespaces are not a safeguard against bad code? So I don't see why > > making it simple for the regular users should be the wrong choice. > > > > Actually I think this is very elegant because this way all needs to use > > these symbols (i.e. prototype and namespace) are in a single place and > > users just do the #include and get all the preconditions. > > Recent LWN https://lwn.net/Articles/998221/ article describes in more details > what I implied under abuser here. Ok. But I don't see how this supports your case. Independent of where the import for a given namespace happens, you can see the used namespaces in modinfo and that is the only place where suspicious imports can be noted reliably. And MODULE_foo isn't relevant here as the namespace of the pwm-lpss driver combo is used in several modules. I thought the point of namespaces is grouping of symbols and reduction of the set of globally available symbols to speed up module loading. I didn't have "duplicate IMPORT_NS statements to make it harder for bad out-of-tree code" on my radar and that shouldn't be a motivation if the price is that in-tree code faces the same constraints IMHO. And I'm pretty sure, we won't prevent a bad actor from successfully using a symbol they should not, just because they need an include *and* an import. Am I missing something? > > > Besides that, you should have based this on recent changes in the NS area of > > > the module symbols, i.e. module namespaces needs to be provided as string > > > literals. > > > > I coordinated my patch set with the pwm maintainer and he is ok with it > > :-) That's why I wrote "This conflicts with > > ceb8bf2ceaa77fe222fe8fe32cb7789c9099ddf1 that is currently in Linus' > > tree. I'll take care about that." in the cover letter. > > Other subsystems simply backmerged those changes. Note, at any time one may > consider origin/master as an immutable (at the point of a given change). > It might be better (if no urgent need) to wait for rc2 and merge it before > doing other changes. I'm well aware of my options. Thanks Uwe
Attachment:
signature.asc
Description: PGP signature
