| Message ID | 20250116154354.149297-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | [v2,1/1] PM: Revert "Add EXPORT macros for exporting PM functions" | expand |
On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > The introduced macros are not doing what they intend for, namely > they won't eliminate the code when CONFIG_PM=n. I don't think they have ever been expected to eliminate the code then. They just don't export the symbols in that case. > Also there were no users of them for all this time. This actually is a good argument for dropping stuff. > Drop them for good and to avoid possible misleading. > > This reverts commit 41a337b40e983db4f0e1602308109f2b93687a06. > > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: elaborated commit message (Greg), Cc'ed to the original author > include/linux/pm.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/include/linux/pm.h b/include/linux/pm.h > index 08c37b83fea8..5dae93817141 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -384,12 +384,8 @@ const struct dev_pm_ops name = { \ > > #ifdef CONFIG_PM > #define _EXPORT_DEV_PM_OPS(name, license, ns) _EXPORT_PM_OPS(name, license, ns) > -#define EXPORT_PM_FN_GPL(name) EXPORT_SYMBOL_GPL(name) > -#define EXPORT_PM_FN_NS_GPL(name, ns) EXPORT_SYMBOL_NS_GPL(name, "ns") > #else > #define _EXPORT_DEV_PM_OPS(name, license, ns) _DISCARD_PM_OPS(name, license, ns) > -#define EXPORT_PM_FN_GPL(name) > -#define EXPORT_PM_FN_NS_GPL(name, ns) > #endif > > #ifdef CONFIG_PM_SLEEP > --
On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > The introduced macros are not doing what they intend for, namely > > they won't eliminate the code when CONFIG_PM=n. > > I don't think they have ever been expected to eliminate the code then. > They just don't export the symbols in that case. Then I'm really puzzled with (potential) usefulness of them to begin with. Having a dead code that is not exported is doubtful benefit. > > Also there were no users of them for all this time. > > This actually is a good argument for dropping stuff. > > > Drop them for good and to avoid possible misleading.
On Thu, Jan 16, 2025 at 5:13 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > The introduced macros are not doing what they intend for, namely > > > they won't eliminate the code when CONFIG_PM=n. > > > > I don't think they have ever been expected to eliminate the code then. > > They just don't export the symbols in that case. > > Then I'm really puzzled with (potential) usefulness of them to begin with. > Having a dead code that is not exported is doubtful benefit. Arguably, exported dead code is even worse. Anyway, it is hard to say what they are good for if there are no users. My point really is that you don't need to add anything beyond "this stuff has no users" to get it removed and arguing about what the unused stuff was intended for is not very useful so to speak.
On Thu, Jan 16, 2025 at 05:27:59PM +0100, Rafael J. Wysocki wrote: > On Thu, Jan 16, 2025 at 5:13 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > The introduced macros are not doing what they intend for, namely > > > > they won't eliminate the code when CONFIG_PM=n. > > > > > > I don't think they have ever been expected to eliminate the code then. > > > They just don't export the symbols in that case. > > > > Then I'm really puzzled with (potential) usefulness of them to begin with. > > Having a dead code that is not exported is doubtful benefit. > > Arguably, exported dead code is even worse. > > Anyway, it is hard to say what they are good for if there are no users. > > My point really is that you don't need to add anything beyond "this > stuff has no users" to get it removed and arguing about what the > unused stuff was intended for is not very useful so to speak. I see. Shall I send a v3 with the reduced commit message?
On 16/01/2025 4:09 pm, Rafael J. Wysocki wrote: > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: >> >> The introduced macros are not doing what they intend for, namely >> they won't eliminate the code when CONFIG_PM=n. > > I don't think they have ever been expected to eliminate the code then. > They just don't export the symbols in that case. > >> Also there were no users of them for all this time. > I had code changes to use them but they got lost at the bottom of a long backlog of other commits and have never been upstreamed. Removing these macros is fine with me. (apologies if you get this msg twice, there was a problem with email) > This actually is a good argument for dropping stuff. > >> Drop them for good and to avoid possible misleading. >> >> This reverts commit 41a337b40e983db4f0e1602308109f2b93687a06. >> >> Reported-by: Adrian Hunter <adrian.hunter@intel.com> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> v2: elaborated commit message (Greg), Cc'ed to the original author >> include/linux/pm.h | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/include/linux/pm.h b/include/linux/pm.h >> index 08c37b83fea8..5dae93817141 100644 >> --- a/include/linux/pm.h >> +++ b/include/linux/pm.h >> @@ -384,12 +384,8 @@ const struct dev_pm_ops name = { \ >> >> #ifdef CONFIG_PM >> #define _EXPORT_DEV_PM_OPS(name, license, ns) _EXPORT_PM_OPS(name, license, ns) >> -#define EXPORT_PM_FN_GPL(name) EXPORT_SYMBOL_GPL(name) >> -#define EXPORT_PM_FN_NS_GPL(name, ns) EXPORT_SYMBOL_NS_GPL(name, "ns") >> #else >> #define _EXPORT_DEV_PM_OPS(name, license, ns) _DISCARD_PM_OPS(name, license, ns) >> -#define EXPORT_PM_FN_GPL(name) >> -#define EXPORT_PM_FN_NS_GPL(name, ns) >> #endif >> >> #ifdef CONFIG_PM_SLEEP >> --
On Thu, Jan 16, 2025 at 5:33 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 16, 2025 at 05:27:59PM +0100, Rafael J. Wysocki wrote: > > On Thu, Jan 16, 2025 at 5:13 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > > > The introduced macros are not doing what they intend for, namely > > > > > they won't eliminate the code when CONFIG_PM=n. > > > > > > > > I don't think they have ever been expected to eliminate the code then. > > > > They just don't export the symbols in that case. > > > > > > Then I'm really puzzled with (potential) usefulness of them to begin with. > > > Having a dead code that is not exported is doubtful benefit. > > > > Arguably, exported dead code is even worse. > > > > Anyway, it is hard to say what they are good for if there are no users. > > > > My point really is that you don't need to add anything beyond "this > > stuff has no users" to get it removed and arguing about what the > > unused stuff was intended for is not very useful so to speak. > > I see. Shall I send a v3 with the reduced commit message? It's there in my queue and I can take care of the changelog, so no need. Thanks!
On Thu, Jan 16, 2025 at 08:18:19PM +0100, Rafael J. Wysocki wrote: > On Thu, Jan 16, 2025 at 5:33 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 16, 2025 at 05:27:59PM +0100, Rafael J. Wysocki wrote: > > > On Thu, Jan 16, 2025 at 5:13 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > > > > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > > > > > The introduced macros are not doing what they intend for, namely > > > > > > they won't eliminate the code when CONFIG_PM=n. > > > > > > > > > > I don't think they have ever been expected to eliminate the code then. > > > > > They just don't export the symbols in that case. > > > > > > > > Then I'm really puzzled with (potential) usefulness of them to begin with. > > > > Having a dead code that is not exported is doubtful benefit. > > > > > > Arguably, exported dead code is even worse. > > > > > > Anyway, it is hard to say what they are good for if there are no users. > > > > > > My point really is that you don't need to add anything beyond "this > > > stuff has no users" to get it removed and arguing about what the > > > unused stuff was intended for is not very useful so to speak. > > > > I see. Shall I send a v3 with the reduced commit message? > > It's there in my queue and I can take care of the changelog, so no need. Ah, thanks!
On Thu, Jan 16, 2025 at 04:55:05PM +0000, Richard Fitzgerald wrote: > On 16/01/2025 4:09 pm, Rafael J. Wysocki wrote: > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > The introduced macros are not doing what they intend for, namely > > > they won't eliminate the code when CONFIG_PM=n. > > > > I don't think they have ever been expected to eliminate the code then. > > They just don't export the symbols in that case. > > > > > Also there were no users of them for all this time. > > I had code changes to use them but they got lost at the bottom of a long > backlog of other commits and have never been upstreamed. Removing these > macros is fine with me. I see, thanks for chiming in in both email threads! > (apologies if you get this msg twice, there was a problem with email)
On Fri, Jan 17, 2025 at 2:39 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 16, 2025 at 08:18:19PM +0100, Rafael J. Wysocki wrote: > > On Thu, Jan 16, 2025 at 5:33 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, Jan 16, 2025 at 05:27:59PM +0100, Rafael J. Wysocki wrote: > > > > On Thu, Jan 16, 2025 at 5:13 PM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > On Thu, Jan 16, 2025 at 05:09:29PM +0100, Rafael J. Wysocki wrote: > > > > > > On Thu, Jan 16, 2025 at 4:44 PM Andy Shevchenko > > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > > > > > > > The introduced macros are not doing what they intend for, namely > > > > > > > they won't eliminate the code when CONFIG_PM=n. > > > > > > > > > > > > I don't think they have ever been expected to eliminate the code then. > > > > > > They just don't export the symbols in that case. > > > > > > > > > > Then I'm really puzzled with (potential) usefulness of them to begin with. > > > > > Having a dead code that is not exported is doubtful benefit. > > > > > > > > Arguably, exported dead code is even worse. > > > > > > > > Anyway, it is hard to say what they are good for if there are no users. > > > > > > > > My point really is that you don't need to add anything beyond "this > > > > stuff has no users" to get it removed and arguing about what the > > > > unused stuff was intended for is not very useful so to speak. > > > > > > I see. Shall I send a v3 with the reduced commit message? > > > > It's there in my queue and I can take care of the changelog, so no need. > > Ah, thanks! Applied now, thanks!
diff --git a/include/linux/pm.h b/include/linux/pm.h index 08c37b83fea8..5dae93817141 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -384,12 +384,8 @@ const struct dev_pm_ops name = { \ #ifdef CONFIG_PM #define _EXPORT_DEV_PM_OPS(name, license, ns) _EXPORT_PM_OPS(name, license, ns) -#define EXPORT_PM_FN_GPL(name) EXPORT_SYMBOL_GPL(name) -#define EXPORT_PM_FN_NS_GPL(name, ns) EXPORT_SYMBOL_NS_GPL(name, "ns") #else #define _EXPORT_DEV_PM_OPS(name, license, ns) _DISCARD_PM_OPS(name, license, ns) -#define EXPORT_PM_FN_GPL(name) -#define EXPORT_PM_FN_NS_GPL(name, ns) #endif #ifdef CONFIG_PM_SLEEP
The introduced macros are not doing what they intend for, namely they won't eliminate the code when CONFIG_PM=n. Also there were no users of them for all this time. Drop them for good and to avoid possible misleading. This reverts commit 41a337b40e983db4f0e1602308109f2b93687a06. Reported-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v2: elaborated commit message (Greg), Cc'ed to the original author include/linux/pm.h | 4 ---- 1 file changed, 4 deletions(-)