| Message ID | 20240606075846.1307007-1-pengfuyuan@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/komeda: Compile DEFINE_SHOW_ATTRIBUTE() only when CONFIG_DEBUG_FS is enabled | expand |
On Thu, 06 Jun 2024, pengfuyuan <pengfuyuan@kylinos.cn> wrote: > We do not call komeda_debugfs_init() and the debugfs core function > declaration if CONFIG_DEBUG_FS is not defined, but we should not > compile it either because the debugfs core function declaration is > not included. > > Reported-by: k2ci <kernel-bot@kylinos.cn> > Signed-off-by: pengfuyuan <pengfuyuan@kylinos.cn> An interesting alternative might actually be to remove *all* the CONFIG_DEBUG_FS conditional compilation from the file. Since the debugfs functions have no-op stubs for CONFIG_DEBUG_FS=n, the compiler will optimize the rest away, because they're no longer referenced. (For the same reason, I don't think this patch has an impact for code size.) The upside for removing conditional compilation is that you'll actually do build testing for both CONFIG_DEBUG_FS config values. Assuming most developers have it enabled, there's not a lot of testing going on for CONFIG_DEBUG_FS=n, and you might introduce build failures with the conditional compilation. Of course, up to Liviu to decide. BR, Jani. > --- > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > index 14ee79becacb..7ada8e6f407c 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > @@ -21,6 +21,7 @@ > > #include "komeda_dev.h" > > +#ifdef CONFIG_DEBUG_FS > static int komeda_register_show(struct seq_file *sf, void *x) > { > struct komeda_dev *mdev = sf->private; > @@ -43,7 +44,6 @@ static int komeda_register_show(struct seq_file *sf, void *x) > > DEFINE_SHOW_ATTRIBUTE(komeda_register); > > -#ifdef CONFIG_DEBUG_FS > static void komeda_debugfs_init(struct komeda_dev *mdev) > { > if (!debugfs_initialized())
On Thu, Jun 06, 2024 at 11:20:58AM +0300, Jani Nikula wrote: > On Thu, 06 Jun 2024, pengfuyuan <pengfuyuan@kylinos.cn> wrote: > > We do not call komeda_debugfs_init() and the debugfs core function > > declaration if CONFIG_DEBUG_FS is not defined, but we should not > > compile it either because the debugfs core function declaration is > > not included. > > > > Reported-by: k2ci <kernel-bot@kylinos.cn> Can we see what the bot reported? > > Signed-off-by: pengfuyuan <pengfuyuan@kylinos.cn> > > An interesting alternative might actually be to remove *all* the > CONFIG_DEBUG_FS conditional compilation from the file. Since the debugfs > functions have no-op stubs for CONFIG_DEBUG_FS=n, the compiler will > optimize the rest away, because they're no longer referenced. (For the > same reason, I don't think this patch has an impact for code size.) > > The upside for removing conditional compilation is that you'll actually > do build testing for both CONFIG_DEBUG_FS config values. Assuming most > developers have it enabled, there's not a lot of testing going on for > CONFIG_DEBUG_FS=n, and you might introduce build failures with the > conditional compilation. > > Of course, up to Liviu to decide. Yeah, I quite like the idea of removing the conditional compilation from the file entirely. Pengfuyuan, do you mind sending a new patch removing all the CONFIG_DEBUG_FS from the file, rather than moving things around? Best regards, Liviu > > > BR, > Jani. > > > > --- > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > index 14ee79becacb..7ada8e6f407c 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > @@ -21,6 +21,7 @@ > > > > #include "komeda_dev.h" > > > > +#ifdef CONFIG_DEBUG_FS > > static int komeda_register_show(struct seq_file *sf, void *x) > > { > > struct komeda_dev *mdev = sf->private; > > @@ -43,7 +44,6 @@ static int komeda_register_show(struct seq_file *sf, void *x) > > > > DEFINE_SHOW_ATTRIBUTE(komeda_register); > > > > -#ifdef CONFIG_DEBUG_FS > > static void komeda_debugfs_init(struct komeda_dev *mdev) > > { > > if (!debugfs_initialized()) > > -- > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c index 14ee79becacb..7ada8e6f407c 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c @@ -21,6 +21,7 @@ #include "komeda_dev.h" +#ifdef CONFIG_DEBUG_FS static int komeda_register_show(struct seq_file *sf, void *x) { struct komeda_dev *mdev = sf->private; @@ -43,7 +44,6 @@ static int komeda_register_show(struct seq_file *sf, void *x) DEFINE_SHOW_ATTRIBUTE(komeda_register); -#ifdef CONFIG_DEBUG_FS static void komeda_debugfs_init(struct komeda_dev *mdev) { if (!debugfs_initialized())
We do not call komeda_debugfs_init() and the debugfs core function declaration if CONFIG_DEBUG_FS is not defined, but we should not compile it either because the debugfs core function declaration is not included. Reported-by: k2ci <kernel-bot@kylinos.cn> Signed-off-by: pengfuyuan <pengfuyuan@kylinos.cn> --- drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)