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

["luo.liu.linux" <luo.liu.linux@163.com>]

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