[PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[luo.liu]

All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
bloating the kernel when debugfs is disabled.

Signed-off-by: luo.liu <luo.liu.linux@163.com>
---
 drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 1c08bba9ecb9..f6a1a57149ba 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
 		return "nil";
 }
 
+#ifdef CONFIG_DEBUG_FS
 static int pending_subdevs_show(struct seq_file *s, void *data)
 {
 	struct v4l2_async_notifier *notif;
@@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
 DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
 
 static struct dentry *v4l2_async_debugfs_dir;
+#endif
 
 static int __init v4l2_async_init(void)
 {
+#ifdef CONFIG_DEBUG_FS
 	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
 	debugfs_create_file("pending_async_subdevices", 0444,
 			    v4l2_async_debugfs_dir, NULL,
 			    &pending_subdevs_fops);
 
+#endif
 	return 0;
 }
 
 static void __exit v4l2_async_exit(void)
 {
+#ifdef CONFIG_DEBUG_FS
 	debugfs_remove_recursive(v4l2_async_debugfs_dir);
+#endif
 }
 
 subsys_initcall(v4l2_async_init);

base-commit: d08c85ac8894995d4b0d8fb48d2f6a3e53cd79ab
-- 
2.25.1

Re:[PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[luo.liu.linux]

 
 Hello Sakari and Mauro，
  
     Gentle ping ....


At 2026-01-21 11:14:56, "luo.liu" <luo.liu.linux@163.com> wrote:
>All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
>bloating the kernel when debugfs is disabled.
>
>Signed-off-by: luo.liu <luo.liu.linux@163.com>
>---
> drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>index 1c08bba9ecb9..f6a1a57149ba 100644
>--- a/drivers/media/v4l2-core/v4l2-async.c
>+++ b/drivers/media/v4l2-core/v4l2-async.c
>@@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
> 		return "nil";
> }
> 
>+#ifdef CONFIG_DEBUG_FS
> static int pending_subdevs_show(struct seq_file *s, void *data)
> {
> 	struct v4l2_async_notifier *notif;
>@@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
> DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
> 
> static struct dentry *v4l2_async_debugfs_dir;
>+#endif
> 
> static int __init v4l2_async_init(void)
> {
>+#ifdef CONFIG_DEBUG_FS
> 	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
> 	debugfs_create_file("pending_async_subdevices", 0444,
> 			    v4l2_async_debugfs_dir, NULL,
> 			    &pending_subdevs_fops);
> 
>+#endif
> 	return 0;
> }
> 
> static void __exit v4l2_async_exit(void)
> {
>+#ifdef CONFIG_DEBUG_FS
> 	debugfs_remove_recursive(v4l2_async_debugfs_dir);
>+#endif
> }
> 
> subsys_initcall(v4l2_async_init);
>
>base-commit: d08c85ac8894995d4b0d8fb48d2f6a3e53cd79ab
>-- 
>2.25.1

Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[Sakari Ailus]

Hi Luo,

Thanks for the patch.

On Wed, Jan 21, 2026 at 11:14:56AM +0800, luo.liu wrote:
> All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
> bloating the kernel when debugfs is disabled.
> 
> Signed-off-by: luo.liu <luo.liu.linux@163.com>
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 1c08bba9ecb9..f6a1a57149ba 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
>  		return "nil";
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
>  static int pending_subdevs_show(struct seq_file *s, void *data)
>  {
>  	struct v4l2_async_notifier *notif;
> @@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
>  DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
>  
>  static struct dentry *v4l2_async_debugfs_dir;
> +#endif

This part seems reasonable...

>  
>  static int __init v4l2_async_init(void)
>  {
> +#ifdef CONFIG_DEBUG_FS
>  	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
>  	debugfs_create_file("pending_async_subdevices", 0444,
>  			    v4l2_async_debugfs_dir, NULL,
>  			    &pending_subdevs_fops);
>  
> +#endif
>  	return 0;
>  }
>  
>  static void __exit v4l2_async_exit(void)
>  {
> +#ifdef CONFIG_DEBUG_FS
>  	debugfs_remove_recursive(v4l2_async_debugfs_dir);
> +#endif
>  }

But are the two latter #if's useful? Looks like the compiler should
optimise these out...

>  
>  subsys_initcall(v4l2_async_init);
> 
> base-commit: d08c85ac8894995d4b0d8fb48d2f6a3e53cd79ab

-- 
Kind regards,

Sakari Ailus

Re:Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[luo.liu.linux]

Hi Sakari,

         Thank you very much for your reply and reminder.

 I think  two latter #if's  is useful，While modern compilers (such as GCC and Clang) possess "Dead Code Elimination" (DCE) capabilities and can optimize away function calls when CONFIG_DEBUG_FS is disabled, 

 explicitly wrapping the relevant code blocks with #ifdef CONFIG_DEBUG_FS remains a standard and necessary practice in Linux kernel development.


Here are the detailed reasons:

1. Prevention of Compilation Errors (Primary Reason)

If CONFIG_DEBUG_FS is not enabled:

Most functions in <linux/debugfs.h> (e.g., debugfs_create_dir, debugfs_create_file, debugfs_remove_recursive) are typically defined as empty inline functions or macros that return NULL or perform no operation.

However, certain data structures referenced in the code or non-inline helper functions might not exist at all.

Crucially, the functions defined in this patch, such as pending_subdevs_show and the macro DEFINE_SHOW_ATTRIBUTE(pending_subdevs), heavily rely on debugfs-specific internal structures (e.g., interactions involving struct seq_file).

Even if v4l2_async_init does not call these functions when the config is disabled, the compiler still attempts to compile their definitions.

If the body of these functions uses APIs or structure members that only exist when CONFIG_DEBUG_FS=y, the compilation will fail (reporting undefined references or missing structure members) rather than failing at the link stage. 

Explicit wrapping ensures that the function bodies, which depend on debugfs internals, are never compiled in the first place.


2. Code Readability and Maintainability

Clear Intent: #ifdef CONFIG_DEBUG_FS clearly signals to anyone reading the code: "This code exists only when the debug filesystem is enabled."

Reduced Noise: When developers disable debugfs to trim the kernel, they expect the related code to disappear completely from the build, rather than seeing a bunch of empty function definitions that were optimized away by the compiler. This helps in understanding the actual composition of the kernel image.

3. Prevention of Potential Side Effects

Although in this simple example the variable v4l2_async_debugfs_dir would only occupy a pointer's space (and likely be optimized out) if unused, in more complex scenarios, unprotected code could execute unnecessary initialization logic or consume static memory, even if ultimately never called.

Kind regards, 

Luo.Liu




At 2026-03-11 15:04:28, "Sakari Ailus" <sakari.ailus@linux.intel.com> wrote:
>Hi Luo,
>
>Thanks for the patch.
>
>On Wed, Jan 21, 2026 at 11:14:56AM +0800, luo.liu wrote:
>> All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
>> bloating the kernel when debugfs is disabled.
>> 
>> Signed-off-by: luo.liu <luo.liu.linux@163.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>> index 1c08bba9ecb9..f6a1a57149ba 100644
>> --- a/drivers/media/v4l2-core/v4l2-async.c
>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>> @@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
>>  		return "nil";
>>  }
>>  
>> +#ifdef CONFIG_DEBUG_FS
>>  static int pending_subdevs_show(struct seq_file *s, void *data)
>>  {
>>  	struct v4l2_async_notifier *notif;
>> @@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
>>  DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
>>  
>>  static struct dentry *v4l2_async_debugfs_dir;
>> +#endif
>
>This part seems reasonable...
>
>>  
>>  static int __init v4l2_async_init(void)
>>  {
>> +#ifdef CONFIG_DEBUG_FS
>>  	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
>>  	debugfs_create_file("pending_async_subdevices", 0444,
>>  			    v4l2_async_debugfs_dir, NULL,
>>  			    &pending_subdevs_fops);
>>  
>> +#endif
>>  	return 0;
>>  }
>>  
>>  static void __exit v4l2_async_exit(void)
>>  {
>> +#ifdef CONFIG_DEBUG_FS
>>  	debugfs_remove_recursive(v4l2_async_debugfs_dir);
>> +#endif
>>  }
>
>But are the two latter #if's useful? Looks like the compiler should
>optimise these out...
>
>>  
>>  subsys_initcall(v4l2_async_init);
>> 
>> base-commit: d08c85ac8894995d4b0d8fb48d2f6a3e53cd79ab
>
>-- 
>Kind regards,
>
>Sakari Ailus

Re: Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[Sakari Ailus]

On Wed, Mar 11, 2026 at 04:58:26PM +0800, luo.liu.linux wrote:
> 
> Hi Sakari,
> 
>          Thank you very much for your reply and reminder.
> 
>  I think  two latter #if's  is useful，While modern compilers (such as GCC and Clang) possess "Dead Code Elimination" (DCE) capabilities and can optimize away function calls when CONFIG_DEBUG_FS is disabled, 
> 
>  explicitly wrapping the relevant code blocks with #ifdef CONFIG_DEBUG_FS remains a standard and necessary practice in Linux kernel development.

Just fix it, please?

-- 
Sakari Ailus

Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[Sakari Ailus]

On Fri, Mar 06, 2026 at 05:41:17PM +0800, luo.liu.linux wrote:
>  
>  Hello Sakari and Mauro，
>   
>      Gentle ping ....
> 
> 
> At 2026-01-21 11:14:56, "luo.liu" <luo.liu.linux@163.com> wrote:
> >All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
> >bloating the kernel when debugfs is disabled.
> >
> >Signed-off-by: luo.liu <luo.liu.linux@163.com>
> >---
> > drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> >diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> >index 1c08bba9ecb9..f6a1a57149ba 100644
> >--- a/drivers/media/v4l2-core/v4l2-async.c
> >+++ b/drivers/media/v4l2-core/v4l2-async.c
> >@@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
> > 		return "nil";
> > }
> > 
> >+#ifdef CONFIG_DEBUG_FS
> > static int pending_subdevs_show(struct seq_file *s, void *data)
> > {
> > 	struct v4l2_async_notifier *notif;
> >@@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
> > DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
> > 
> > static struct dentry *v4l2_async_debugfs_dir;
> >+#endif
> > 
> > static int __init v4l2_async_init(void)
> > {
> >+#ifdef CONFIG_DEBUG_FS
> > 	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
> > 	debugfs_create_file("pending_async_subdevices", 0444,
> > 			    v4l2_async_debugfs_dir, NULL,
> > 			    &pending_subdevs_fops);
> > 
> >+#endif
> > 	return 0;
> > }
> > 
> > static void __exit v4l2_async_exit(void)
> > {
> >+#ifdef CONFIG_DEBUG_FS
> > 	debugfs_remove_recursive(v4l2_async_debugfs_dir);
> >+#endif
> > }

If debugfs is disabled, the init functions won't do anything. You can put
these behind a single #ifdef .. #endif.

-- 
Sakari Ailus

Re:Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[luo.liu.linux]

Hi Sakari,

        Thank you very much for your review and suggestions.


          Yes, I originally intended to wrap everything in a single #ifdef CONFIG_DEBUG_FS ... #endif block. However, since v4l2_async_init and v4l2_async_exit are mandatory entry points (registered via subsys_initcall and module_exit), 

they must remain present regardless of the configuration. This limits the granularity, forcing me to place the conditional checks inside these two functions.


Kind regards, 

Luo.Liu


At 2026-03-11 17:20:41, "Sakari Ailus" <sakari.ailus@linux.intel.com> wrote:
>On Fri, Mar 06, 2026 at 05:41:17PM +0800, luo.liu.linux wrote:
>>  
>>  Hello Sakari and Mauro，
>>   
>>      Gentle ping ....
>> 
>> 
>> At 2026-01-21 11:14:56, "luo.liu" <luo.liu.linux@163.com> wrote:
>> >All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
>> >bloating the kernel when debugfs is disabled.
>> >
>> >Signed-off-by: luo.liu <luo.liu.linux@163.com>
>> >---
>> > drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> >diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>> >index 1c08bba9ecb9..f6a1a57149ba 100644
>> >--- a/drivers/media/v4l2-core/v4l2-async.c
>> >+++ b/drivers/media/v4l2-core/v4l2-async.c
>> >@@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
>> > 		return "nil";
>> > }
>> > 
>> >+#ifdef CONFIG_DEBUG_FS
>> > static int pending_subdevs_show(struct seq_file *s, void *data)
>> > {
>> > 	struct v4l2_async_notifier *notif;
>> >@@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
>> > DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
>> > 
>> > static struct dentry *v4l2_async_debugfs_dir;
>> >+#endif
>> > 
>> > static int __init v4l2_async_init(void)
>> > {
>> >+#ifdef CONFIG_DEBUG_FS
>> > 	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
>> > 	debugfs_create_file("pending_async_subdevices", 0444,
>> > 			    v4l2_async_debugfs_dir, NULL,
>> > 			    &pending_subdevs_fops);
>> > 
>> >+#endif
>> > 	return 0;
>> > }
>> > 
>> > static void __exit v4l2_async_exit(void)
>> > {
>> >+#ifdef CONFIG_DEBUG_FS
>> > 	debugfs_remove_recursive(v4l2_async_debugfs_dir);
>> >+#endif
>> > }
>
>If debugfs is disabled, the init functions won't do anything. You can put
>these behind a single #ifdef .. #endif.
>
>-- 
>Sakari Ailus

Re:Re: [PATCH v2] media:v4l2-async:add debugfs under CONFIG_DEBUG_FS

[luo.liu.linux]

Hi Sakari,

     Thank you very much for your review and suggestions.
  
     I considered wrapping the entire block in a single #ifdef, but since v4l2_async_init and v4l2_async_exit are mandatory module entry points, they cannot be omitted even when CONFIG_DEBUG_FS is disabled.
 Therefore, the #ifdef guards inside these functions are necessary to prevent references to undefined symbols (like pending_subdevs_fops and v4l2_async_debugfs_dir) while keeping the function definitions themselves intact

     I have consolidated all debugfs-specific definitions (the show function and the directory pointer) into one block to keep the code clean, leaving only the minimal necessary guards inside the init/exit functions






 
















At 2026-03-11 17:20:41, "Sakari Ailus" <sakari.ailus@linux.intel.com> wrote:
>On Fri, Mar 06, 2026 at 05:41:17PM +0800, luo.liu.linux wrote:
>>  
>>  Hello Sakari and Mauro，
>>   
>>      Gentle ping ....
>> 
>> 
>> At 2026-01-21 11:14:56, "luo.liu" <luo.liu.linux@163.com> wrote:
>> >All debugfs-related code is guarded by CONFIG_DEBUG_FS to avoid
>> >bloating the kernel when debugfs is disabled.
>> >
>> >Signed-off-by: luo.liu <luo.liu.linux@163.com>
>> >---
>> > drivers/media/v4l2-core/v4l2-async.c | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> >diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>> >index 1c08bba9ecb9..f6a1a57149ba 100644
>> >--- a/drivers/media/v4l2-core/v4l2-async.c
>> >+++ b/drivers/media/v4l2-core/v4l2-async.c
>> >@@ -947,6 +947,7 @@ v4l2_async_nf_name(struct v4l2_async_notifier *notifier)
>> > 		return "nil";
>> > }
>> > 
>> >+#ifdef CONFIG_DEBUG_FS
>> > static int pending_subdevs_show(struct seq_file *s, void *data)
>> > {
>> > 	struct v4l2_async_notifier *notif;
>> >@@ -967,20 +968,25 @@ static int pending_subdevs_show(struct seq_file *s, void *data)
>> > DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
>> > 
>> > static struct dentry *v4l2_async_debugfs_dir;
>> >+#endif
>> > 
>> > static int __init v4l2_async_init(void)
>> > {
>> >+#ifdef CONFIG_DEBUG_FS
>> > 	v4l2_async_debugfs_dir = debugfs_create_dir("v4l2-async", NULL);
>> > 	debugfs_create_file("pending_async_subdevices", 0444,
>> > 			    v4l2_async_debugfs_dir, NULL,
>> > 			    &pending_subdevs_fops);
>> > 
>> >+#endif
>> > 	return 0;
>> > }
>> > 
>> > static void __exit v4l2_async_exit(void)
>> > {
>> >+#ifdef CONFIG_DEBUG_FS
>> > 	debugfs_remove_recursive(v4l2_async_debugfs_dir);
>> >+#endif
>> > }
>
>If debugfs is disabled, the init functions won't do anything. You can put
>these behind a single #ifdef .. #endif.
>
>-- 
>Sakari Ailus