On Mon, 3 Jan 2022 12:58:46 -0500 Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Mon, Jan 3, 2022 at 10:24 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Sun, 2 Jan 2022 09:15:06 -0500 Arnd Bergmann <arnd@xxxxxxxx> wrote: > > > > That came up in discussion of the patch series introducing this macro > > https://lore.kernel.org/linux-pm/20211216110936.6ccd07d3@jic23-huawei/ > > > > Most of the cases that don't define it as static > > also export the symbol for use by other modules so the approach of letting > > the compiler detect it as dead code won't always work. > > > > Exceptions from a bit of grepping are > > net/ethernet/broadcom/bnx2x/ > > scsi/esas2r/esas2r_init.c > > not sure on reasoning behind the file splits in these drivers > > but definitely looks like it we could just merge a few files > > and let this be static + the compiler remove it neatly. > > > > vs 17 cases where the symbol is exported and more cleverness will > > be needed. > > I don't see why exporting the symbol makes a difference at all, either > it is needed in another file or it is not. Ah. My reasoning was that the purpose behind that patch set (letting compiler remove the functions etc when unused) was not applicable in EXPORT* cases. However, I'd neglected that the DEFINE_SIMPLE_DEV_PM_OPS() macro is probably useful anyway as those cases would need CONFIG_PM* guards if they want to remove the code when PM stuff isn't enabled. > > I think it would be more natural to not include 'static' in the macro, > that is certainly what all other macros like this do, and it's still > trivial to add 'static' manually, but impossible to remove it. IIRC there are other cases like this, but it was exactly this somewhat unusual element that made me raise the question in the original discussion. Paul, over to you for reasoning. If we are going to change this now is the time before they get significant use and we end up having to add static to lots of places. Jonathan > > Arnd