[PATCHES] assorted debugfs stuff

[PATCHES] assorted debugfs stuff

[Al Viro]

	A bit more of debugfs work; that stuff sits in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.debugfs
Individual patches in followups.  Please, review.

Several removals of pointless debugfs_file_{get,put}():
      zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops
      hfi1: get rid of redundant debugfs_file_{get,put}()
      regmap: get rid of redundant debugfs_file_{get,put}()
      resctrl: get rid of pointless debugfs_file_{get,put}()
Getting rid of the last remnants of debugfs_real_fops():
      vmscan: don't bother with debugfs_real_fops()
      netronome: don't bother with debugfs_real_fops()
      debugfs: split short and full proxy wrappers, kill debugfs_real_fops()

Bogosities in drivers/thermal/testing:
      fix tt_command_write():
1) unbalanced debugfs_file_get().  Not needed in the first place -
file_operations are accessed only via debugfs_create_file(), so
debugfs wrappers will take care of that itself.
2) kmalloc() for a buffer used only for duration of a function is not
a problem, but for a buffer no longer than 16 bytes?
3) strstr() is for finding substrings; for finding a character there's
strchr().

debugfs_get_aux() stuff:
      debugfs_get_aux(): allow storing non-const void *
      blk-mq-debugfs: use debugfs_aux_data()
      lpfc: don't use file->f_path.dentry for comparisons
If you want a home-grown switch, at least use enum for selector...

[PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Al Viro]

When debugfs file has been created by debugfs_create_file_unsafe(),
we do need the file_operations methods to use debugfs_file_{get,put}()
to prevent concurrent removal; for files created by debugfs_create_file()
that is done in the wrappers that call underlying methods, so there's
no point whatsoever duplicating that in the underlying methods themselves.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/gpu/drm/xlnx/zynqmp_dp.c | 38 ++++----------------------------
 1 file changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 238cbb49963e..197defe4f928 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1869,20 +1869,14 @@ static int zynqmp_dp_test_setup(struct zynqmp_dp *dp)
 static ssize_t zynqmp_dp_pattern_read(struct file *file, char __user *user_buf,
 				      size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	struct zynqmp_dp *dp = file->private_data;
 	char buf[16];
 	ssize_t ret;
 
-	ret = debugfs_file_get(dentry);
-	if (unlikely(ret))
-		return ret;
-
 	scoped_guard(mutex, &dp->lock)
 		ret = snprintf(buf, sizeof(buf), "%s\n",
 			       test_pattern_str[dp->test.pattern]);
 
-	debugfs_file_put(dentry);
 	return simple_read_from_buffer(user_buf, count, ppos, buf, ret);
 }
 
@@ -1890,27 +1884,20 @@ static ssize_t zynqmp_dp_pattern_write(struct file *file,
 				       const char __user *user_buf,
 				       size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	struct zynqmp_dp *dp = file->private_data;
 	char buf[16];
 	ssize_t ret;
 	int pattern;
 
-	ret = debugfs_file_get(dentry);
-	if (unlikely(ret))
-		return ret;
-
 	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
 				     count);
 	if (ret < 0)
-		goto out;
+		return ret;
 	buf[ret] = '\0';
 
 	pattern = sysfs_match_string(test_pattern_str, buf);
-	if (pattern < 0) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (pattern < 0)
+		return -EINVAL;
 
 	mutex_lock(&dp->lock);
 	dp->test.pattern = pattern;
@@ -1919,8 +1906,6 @@ static ssize_t zynqmp_dp_pattern_write(struct file *file,
 						 dp->test.custom) ?: ret;
 	mutex_unlock(&dp->lock);
 
-out:
-	debugfs_file_put(dentry);
 	return ret;
 }
 
@@ -2026,20 +2011,13 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_active, zynqmp_dp_active_get,
 static ssize_t zynqmp_dp_custom_read(struct file *file, char __user *user_buf,
 				     size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	struct zynqmp_dp *dp = file->private_data;
 	ssize_t ret;
 
-	ret = debugfs_file_get(dentry);
-	if (unlikely(ret))
-		return ret;
-
 	mutex_lock(&dp->lock);
 	ret = simple_read_from_buffer(user_buf, count, ppos, &dp->test.custom,
 				      sizeof(dp->test.custom));
 	mutex_unlock(&dp->lock);
-
-	debugfs_file_put(dentry);
 	return ret;
 }
 
@@ -2047,18 +2025,13 @@ static ssize_t zynqmp_dp_custom_write(struct file *file,
 				      const char __user *user_buf,
 				      size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	struct zynqmp_dp *dp = file->private_data;
 	ssize_t ret;
 	char buf[sizeof(dp->test.custom)];
 
-	ret = debugfs_file_get(dentry);
-	if (unlikely(ret))
-		return ret;
-
 	ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	mutex_lock(&dp->lock);
 	memcpy(dp->test.custom, buf, ret);
@@ -2066,9 +2039,6 @@ static ssize_t zynqmp_dp_custom_write(struct file *file,
 		ret = zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
 						 dp->test.custom) ?: ret;
 	mutex_unlock(&dp->lock);
-
-out:
-	debugfs_file_put(dentry);
 	return ret;
 }
 
-- 
2.39.5

[PATCH 02/11] hfi1: get rid of redundant debugfs_file_{get,put}()

[Al Viro]

All files in question are created via debugfs_create_file(), so
exclusion with removals is provided by debugfs wrappers; as the matter
of fact, hfi1-private wrappers had been redundant at least since 2017...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/infiniband/hw/hfi1/debugfs.c | 28 ----------------------------
 drivers/infiniband/hw/hfi1/debugfs.h |  9 ++-------
 drivers/infiniband/hw/hfi1/fault.c   |  9 ---------
 3 files changed, 2 insertions(+), 44 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index a1e01b447265..ac37ab7f8995 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -22,34 +22,6 @@
 
 static struct dentry *hfi1_dbg_root;
 
-/* wrappers to enforce srcu in seq file */
-ssize_t hfi1_seq_read(struct file *file, char __user *buf, size_t size,
-		      loff_t *ppos)
-{
-	struct dentry *d = file->f_path.dentry;
-	ssize_t r;
-
-	r = debugfs_file_get(d);
-	if (unlikely(r))
-		return r;
-	r = seq_read(file, buf, size, ppos);
-	debugfs_file_put(d);
-	return r;
-}
-
-loff_t hfi1_seq_lseek(struct file *file, loff_t offset, int whence)
-{
-	struct dentry *d = file->f_path.dentry;
-	loff_t r;
-
-	r = debugfs_file_get(d);
-	if (unlikely(r))
-		return r;
-	r = seq_lseek(file, offset, whence);
-	debugfs_file_put(d);
-	return r;
-}
-
 #define private2dd(file) (file_inode(file)->i_private)
 #define private2ppd(file) (file_inode(file)->i_private)
 
diff --git a/drivers/infiniband/hw/hfi1/debugfs.h b/drivers/infiniband/hw/hfi1/debugfs.h
index 54d952a4016c..65b48839abc6 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.h
+++ b/drivers/infiniband/hw/hfi1/debugfs.h
@@ -33,16 +33,11 @@ static int _##name##_open(struct inode *inode, struct file *s) \
 static const struct file_operations _##name##_file_ops = { \
 	.owner   = THIS_MODULE, \
 	.open    = _##name##_open, \
-	.read    = hfi1_seq_read, \
-	.llseek  = hfi1_seq_lseek, \
+	.read    = seq_read, \
+	.llseek  = seq_lseek, \
 	.release = seq_release \
 }
 
-
-ssize_t hfi1_seq_read(struct file *file, char __user *buf, size_t size,
-		      loff_t *ppos);
-loff_t hfi1_seq_lseek(struct file *file, loff_t offset, int whence);
-
 #ifdef CONFIG_DEBUG_FS
 void hfi1_dbg_ibdev_init(struct hfi1_ibdev *ibd);
 void hfi1_dbg_ibdev_exit(struct hfi1_ibdev *ibd);
diff --git a/drivers/infiniband/hw/hfi1/fault.c b/drivers/infiniband/hw/hfi1/fault.c
index ec9ee59fcf0c..a45cbffd52c7 100644
--- a/drivers/infiniband/hw/hfi1/fault.c
+++ b/drivers/infiniband/hw/hfi1/fault.c
@@ -104,9 +104,6 @@ static ssize_t fault_opcodes_write(struct file *file, const char __user *buf,
 		goto free_data;
 	}
 
-	ret = debugfs_file_get(file->f_path.dentry);
-	if (unlikely(ret))
-		goto free_data;
 	ptr = data;
 	token = ptr;
 	for (ptr = data; *ptr; ptr = end + 1, token = ptr) {
@@ -154,7 +151,6 @@ static ssize_t fault_opcodes_write(struct file *file, const char __user *buf,
 	}
 	ret = len;
 
-	debugfs_file_put(file->f_path.dentry);
 free_data:
 	kfree(data);
 	return ret;
@@ -173,9 +169,6 @@ static ssize_t fault_opcodes_read(struct file *file, char __user *buf,
 	data = kcalloc(datalen, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	ret = debugfs_file_get(file->f_path.dentry);
-	if (unlikely(ret))
-		goto free_data;
 	bit = find_first_bit(fault->opcodes, bitsize);
 	while (bit < bitsize) {
 		zero = find_next_zero_bit(fault->opcodes, bitsize, bit);
@@ -189,11 +182,9 @@ static ssize_t fault_opcodes_read(struct file *file, char __user *buf,
 					 bit);
 		bit = find_next_bit(fault->opcodes, bitsize, zero);
 	}
-	debugfs_file_put(file->f_path.dentry);
 	data[size - 1] = '\n';
 	data[size] = '\0';
 	ret = simple_read_from_buffer(buf, len, pos, data, size);
-free_data:
 	kfree(data);
 	return ret;
 }
-- 
2.39.5

[PATCH 03/11] regmap: get rid of redundant debugfs_file_{get,put}()

[Al Viro]

pointless in ->read()/->write() of file_operations used only via
debugfs_create_file()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/base/regmap/regmap-debugfs.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index fb84cda92a75..c9b4c04b1cf6 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -470,10 +470,6 @@ static ssize_t regmap_cache_only_write_file(struct file *file,
 	if (err)
 		return count;
 
-	err = debugfs_file_get(file->f_path.dentry);
-	if (err)
-		return err;
-
 	map->lock(map->lock_arg);
 
 	if (new_val && !map->cache_only) {
@@ -486,7 +482,6 @@ static ssize_t regmap_cache_only_write_file(struct file *file,
 	map->cache_only = new_val;
 
 	map->unlock(map->lock_arg);
-	debugfs_file_put(file->f_path.dentry);
 
 	if (require_sync) {
 		err = regcache_sync(map);
@@ -517,10 +512,6 @@ static ssize_t regmap_cache_bypass_write_file(struct file *file,
 	if (err)
 		return count;
 
-	err = debugfs_file_get(file->f_path.dentry);
-	if (err)
-		return err;
-
 	map->lock(map->lock_arg);
 
 	if (new_val && !map->cache_bypass) {
@@ -532,7 +523,6 @@ static ssize_t regmap_cache_bypass_write_file(struct file *file,
 	map->cache_bypass = new_val;
 
 	map->unlock(map->lock_arg);
-	debugfs_file_put(file->f_path.dentry);
 
 	return count;
 }
-- 
2.39.5

Re: [PATCH 03/11] regmap: get rid of redundant debugfs_file_{get,put}()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

On Wed, Jul 02, 2025 at 10:16:02PM +0100, Al Viro wrote:
> pointless in ->read()/->write() of file_operations used only via
> debugfs_create_file()

What's the story with dependencies here - does this need to go with the
rest of the series?  I don't have the cover letter or the rest of the
series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Al Viro]

->write() of file_operations that gets used only via debugfs_create_file()
is called only under debugfs_file_get()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/resctrl/pseudo_lock.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/resctrl/pseudo_lock.c b/fs/resctrl/pseudo_lock.c
index ccc2f9213b4b..87bbc2605de1 100644
--- a/fs/resctrl/pseudo_lock.c
+++ b/fs/resctrl/pseudo_lock.c
@@ -764,13 +764,9 @@ static ssize_t pseudo_lock_measure_trigger(struct file *file,
 	if (ret == 0) {
 		if (sel != 1 && sel != 2 && sel != 3)
 			return -EINVAL;
-		ret = debugfs_file_get(file->f_path.dentry);
-		if (ret)
-			return ret;
 		ret = pseudo_lock_measure_cycles(rdtgrp, sel);
 		if (ret == 0)
 			ret = count;
-		debugfs_file_put(file->f_path.dentry);
 	}
 
 	return ret;
-- 
2.39.5

Re: [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Luck, Tony]

On Wed, Jul 02, 2025 at 10:16:50PM +0100, Al Viro wrote:

+Reinette

> ->write() of file_operations that gets used only via debugfs_create_file()
> is called only under debugfs_file_get()
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/resctrl/pseudo_lock.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/fs/resctrl/pseudo_lock.c b/fs/resctrl/pseudo_lock.c
> index ccc2f9213b4b..87bbc2605de1 100644
> --- a/fs/resctrl/pseudo_lock.c
> +++ b/fs/resctrl/pseudo_lock.c
> @@ -764,13 +764,9 @@ static ssize_t pseudo_lock_measure_trigger(struct file *file,
>  	if (ret == 0) {
>  		if (sel != 1 && sel != 2 && sel != 3)
>  			return -EINVAL;
> -		ret = debugfs_file_get(file->f_path.dentry);
> -		if (ret)
> -			return ret;
>  		ret = pseudo_lock_measure_cycles(rdtgrp, sel);
>  		if (ret == 0)
>  			ret = count;
> -		debugfs_file_put(file->f_path.dentry);
>  	}
>  
>  	return ret;
> -- 
> 2.39.5
> 

Re: [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Reinette Chatre]

On 7/2/25 2:24 PM, Luck, Tony wrote:
> On Wed, Jul 02, 2025 at 10:16:50PM +0100, Al Viro wrote:
> 
> +Reinette

Thank you Tony.

> 
>> ->write() of file_operations that gets used only via debugfs_create_file()
>> is called only under debugfs_file_get()
>>
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>> ---
>>  fs/resctrl/pseudo_lock.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/fs/resctrl/pseudo_lock.c b/fs/resctrl/pseudo_lock.c
>> index ccc2f9213b4b..87bbc2605de1 100644
>> --- a/fs/resctrl/pseudo_lock.c
>> +++ b/fs/resctrl/pseudo_lock.c
>> @@ -764,13 +764,9 @@ static ssize_t pseudo_lock_measure_trigger(struct file *file,
>>  	if (ret == 0) {
>>  		if (sel != 1 && sel != 2 && sel != 3)
>>  			return -EINVAL;
>> -		ret = debugfs_file_get(file->f_path.dentry);
>> -		if (ret)
>> -			return ret;
>>  		ret = pseudo_lock_measure_cycles(rdtgrp, sel);
>>  		if (ret == 0)
>>  			ret = count;
>> -		debugfs_file_put(file->f_path.dentry);
>>  	}
>>  
>>  	return ret;
>> -- 
>> 2.39.5
>>

Thank you very much for catching and fixing this Al.

Acked-by: Reinette Chatre <reinette.chatre@intel.com>

How are the patches from this series expected to flow upstream?
resctrl changes usually flow upstream via tip. Would you be ok if
I pick just this patch and route it via tip or would you prefer to
keep it with this series? At this time I do not anticipate
any conflicts if this patch goes upstream via other FS changes during
this cycle.

Reinette

Re: [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Al Viro]

On Wed, Jul 02, 2025 at 04:45:36PM -0700, Reinette Chatre wrote:

> Thank you very much for catching and fixing this Al.
> 
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> 
> How are the patches from this series expected to flow upstream?
> resctrl changes usually flow upstream via tip. Would you be ok if
> I pick just this patch and route it via tip or would you prefer to
> keep it with this series? At this time I do not anticipate
> any conflicts if this patch goes upstream via other FS changes during
> this cycle.

Up to Greg, really...

Re: [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Greg Kroah-Hartman]

On Thu, Jul 03, 2025 at 01:24:19AM +0100, Al Viro wrote:
> On Wed, Jul 02, 2025 at 04:45:36PM -0700, Reinette Chatre wrote:
> 
> > Thank you very much for catching and fixing this Al.
> > 
> > Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> > 
> > How are the patches from this series expected to flow upstream?
> > resctrl changes usually flow upstream via tip. Would you be ok if
> > I pick just this patch and route it via tip or would you prefer to
> > keep it with this series? At this time I do not anticipate
> > any conflicts if this patch goes upstream via other FS changes during
> > this cycle.
> 
> Up to Greg, really...

I'll take them all, give me a day or so to catch up with pending
reviews, thanks.

greg k-h

Re: [PATCH 04/11] resctrl: get rid of pointless debugfs_file_{get,put}()

[Reinette Chatre]

On 7/2/25 10:40 PM, Greg Kroah-Hartman wrote:
> On Thu, Jul 03, 2025 at 01:24:19AM +0100, Al Viro wrote:
>> On Wed, Jul 02, 2025 at 04:45:36PM -0700, Reinette Chatre wrote:
>>
>>> Thank you very much for catching and fixing this Al.
>>>
>>> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
>>>
>>> How are the patches from this series expected to flow upstream?
>>> resctrl changes usually flow upstream via tip. Would you be ok if
>>> I pick just this patch and route it via tip or would you prefer to
>>> keep it with this series? At this time I do not anticipate
>>> any conflicts if this patch goes upstream via other FS changes during
>>> this cycle.
>>
>> Up to Greg, really...
> 
> I'll take them all, give me a day or so to catch up with pending
> reviews, thanks.
> 

Thank you very much Greg.

Reinette

[PATCH 05/11] vmscan: don't bother with debugfs_real_fops()

[Al Viro]

... not when it's used only to check which file is used;
debugfs_create_file_aux_num() allows to stash a number into
debugfs entry and debugfs_get_aux_num() extracts it.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 mm/vmscan.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f8dfd2864bbf..0e661672cbb9 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5420,7 +5420,7 @@ static void lru_gen_seq_show_full(struct seq_file *m, struct lruvec *lruvec,
 static int lru_gen_seq_show(struct seq_file *m, void *v)
 {
 	unsigned long seq;
-	bool full = !debugfs_real_fops(m->file)->write;
+	bool full = debugfs_get_aux_num(m->file);
 	struct lruvec *lruvec = v;
 	struct lru_gen_folio *lrugen = &lruvec->lrugen;
 	int nid = lruvec_pgdat(lruvec)->node_id;
@@ -5756,8 +5756,10 @@ static int __init init_lru_gen(void)
 	if (sysfs_create_group(mm_kobj, &lru_gen_attr_group))
 		pr_err("lru_gen: failed to create sysfs group\n");
 
-	debugfs_create_file("lru_gen", 0644, NULL, NULL, &lru_gen_rw_fops);
-	debugfs_create_file("lru_gen_full", 0444, NULL, NULL, &lru_gen_ro_fops);
+	debugfs_create_file_aux_num("lru_gen", 0644, NULL, NULL, 1,
+				    &lru_gen_rw_fops);
+	debugfs_create_file_aux_num("lru_gen_full", 0444, NULL, NULL, 0,
+				    &lru_gen_ro_fops);
 
 	return 0;
 };
-- 
2.39.5

Re: [PATCH 05/11] vmscan: don't bother with debugfs_real_fops()

[Matthew Wilcox]

On Wed, Jul 02, 2025 at 10:17:39PM +0100, Al Viro wrote:
> -	bool full = !debugfs_real_fops(m->file)->write;
> +	bool full = debugfs_get_aux_num(m->file);

> +	debugfs_create_file_aux_num("lru_gen", 0644, NULL, NULL, 1,
> +				    &lru_gen_rw_fops);
> +	debugfs_create_file_aux_num("lru_gen_full", 0444, NULL, NULL, 0,
> +				    &lru_gen_ro_fops);

Looks like you have the polarity inverted there?

Re: [PATCH 05/11] vmscan: don't bother with debugfs_real_fops()

[Al Viro]

On Thu, Jul 03, 2025 at 12:51:57PM +0100, Matthew Wilcox wrote:
> On Wed, Jul 02, 2025 at 10:17:39PM +0100, Al Viro wrote:
> > -	bool full = !debugfs_real_fops(m->file)->write;
> > +	bool full = debugfs_get_aux_num(m->file);
> 
> > +	debugfs_create_file_aux_num("lru_gen", 0644, NULL, NULL, 1,
> > +				    &lru_gen_rw_fops);
> > +	debugfs_create_file_aux_num("lru_gen_full", 0444, NULL, NULL, 0,
> > +				    &lru_gen_ro_fops);
> 
> Looks like you have the polarity inverted there?

Right you are.  My apologies...  Fixed, force-pushed into the same branch,
replacement commit below:

From 51d26db0fd00fbd501f9271550667bab6c5fb107 Mon Sep 17 00:00:00 2001
From: Al Viro <viro@zeniv.linux.org.uk>
Date: Wed, 29 Jan 2025 14:43:44 -0500
Subject: [PATCH 05/11] vmscan: don't bother with debugfs_real_fops()

... not when it's used only to check which file is used;
debugfs_create_file_aux_num() allows to stash a number into
debugfs entry and debugfs_get_aux_num() extracts it.

Braino-spotted-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 mm/vmscan.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f8dfd2864bbf..27c70848c0a0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5420,7 +5420,7 @@ static void lru_gen_seq_show_full(struct seq_file *m, struct lruvec *lruvec,
 static int lru_gen_seq_show(struct seq_file *m, void *v)
 {
 	unsigned long seq;
-	bool full = !debugfs_real_fops(m->file)->write;
+	bool full = debugfs_get_aux_num(m->file);
 	struct lruvec *lruvec = v;
 	struct lru_gen_folio *lrugen = &lruvec->lrugen;
 	int nid = lruvec_pgdat(lruvec)->node_id;
@@ -5756,8 +5756,10 @@ static int __init init_lru_gen(void)
 	if (sysfs_create_group(mm_kobj, &lru_gen_attr_group))
 		pr_err("lru_gen: failed to create sysfs group\n");
 
-	debugfs_create_file("lru_gen", 0644, NULL, NULL, &lru_gen_rw_fops);
-	debugfs_create_file("lru_gen_full", 0444, NULL, NULL, &lru_gen_ro_fops);
+	debugfs_create_file_aux_num("lru_gen", 0644, NULL, NULL, false,
+				    &lru_gen_rw_fops);
+	debugfs_create_file_aux_num("lru_gen_full", 0444, NULL, NULL, true,
+				    &lru_gen_ro_fops);
 
 	return 0;
 };
-- 
2.39.5

[PATCH 06/11] netronome: don't bother with debugfs_real_fops()

[Al Viro]

Just turn nfp_tx_q_show() into a wrapper for helper that gets
told whether it's tx or xdp via an explicit argument and have
nfp_xdp_q_show() call the underlying helper instead.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 .../net/ethernet/netronome/nfp/nfp_net_debugfs.c  | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
index d8b735ccf899..d843d1e19715 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
@@ -77,7 +77,7 @@ DEFINE_SHOW_ATTRIBUTE(nfp_rx_q);
 static int nfp_tx_q_show(struct seq_file *file, void *data);
 DEFINE_SHOW_ATTRIBUTE(nfp_tx_q);
 
-static int nfp_tx_q_show(struct seq_file *file, void *data)
+static int __nfp_tx_q_show(struct seq_file *file, void *data, bool is_xdp)
 {
 	struct nfp_net_r_vector *r_vec = file->private;
 	struct nfp_net_tx_ring *tx_ring;
@@ -86,10 +86,10 @@ static int nfp_tx_q_show(struct seq_file *file, void *data)
 
 	rtnl_lock();
 
-	if (debugfs_real_fops(file->file) == &nfp_tx_q_fops)
-		tx_ring = r_vec->tx_ring;
-	else
+	if (is_xdp)
 		tx_ring = r_vec->xdp_ring;
+	else
+		tx_ring = r_vec->tx_ring;
 	if (!r_vec->nfp_net || !tx_ring)
 		goto out;
 	nn = r_vec->nfp_net;
@@ -115,9 +115,14 @@ static int nfp_tx_q_show(struct seq_file *file, void *data)
 	return 0;
 }
 
+static int nfp_tx_q_show(struct seq_file *file, void *data)
+{
+	return __nfp_tx_q_show(file, data, false);
+}
+
 static int nfp_xdp_q_show(struct seq_file *file, void *data)
 {
-	return nfp_tx_q_show(file, data);
+	return __nfp_tx_q_show(file, data, true);
 }
 DEFINE_SHOW_ATTRIBUTE(nfp_xdp_q);
 
-- 
2.39.5

Re: [PATCH 06/11] netronome: don't bother with debugfs_real_fops()

[Louis Peens]

On Wed, Jul 02, 2025 at 10:22:05PM +0100, Al Viro wrote:
> Just turn nfp_tx_q_show() into a wrapper for helper that gets
> told whether it's tx or xdp via an explicit argument and have
> nfp_xdp_q_show() call the underlying helper instead.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  .../net/ethernet/netronome/nfp/nfp_net_debugfs.c  | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
> index d8b735ccf899..d843d1e19715 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c
> @@ -77,7 +77,7 @@ DEFINE_SHOW_ATTRIBUTE(nfp_rx_q);
>  static int nfp_tx_q_show(struct seq_file *file, void *data);
>  DEFINE_SHOW_ATTRIBUTE(nfp_tx_q);
I could be missing something, but I think this update now allows this
DEFINE_SHOW_ATTRIBUTE(nfp_tx_q) to move down to below the function
wrapper, and the 'nfp_tx_q_show' declaration here is not needed
anymore?

This is just a tiny nit though, I'm also fine if you want to leave it as is.
This looks like a nice cleanup to me, thanks!

Reviewed-by: Louis Peens <louis.peens@corigine.com>

>  
> -static int nfp_tx_q_show(struct seq_file *file, void *data)
> +static int __nfp_tx_q_show(struct seq_file *file, void *data, bool is_xdp)
>  {
>  	struct nfp_net_r_vector *r_vec = file->private;
>  	struct nfp_net_tx_ring *tx_ring;
> @@ -86,10 +86,10 @@ static int nfp_tx_q_show(struct seq_file *file, void *data)
>  
>  	rtnl_lock();
>  
> -	if (debugfs_real_fops(file->file) == &nfp_tx_q_fops)
> -		tx_ring = r_vec->tx_ring;
> -	else
> +	if (is_xdp)
>  		tx_ring = r_vec->xdp_ring;
> +	else
> +		tx_ring = r_vec->tx_ring;
>  	if (!r_vec->nfp_net || !tx_ring)
>  		goto out;
>  	nn = r_vec->nfp_net;
> @@ -115,9 +115,14 @@ static int nfp_tx_q_show(struct seq_file *file, void *data)
>  	return 0;
>  }
>  
> +static int nfp_tx_q_show(struct seq_file *file, void *data)
> +{
> +	return __nfp_tx_q_show(file, data, false);
> +}
> +
>  static int nfp_xdp_q_show(struct seq_file *file, void *data)
>  {
> -	return nfp_tx_q_show(file, data);
> +	return __nfp_tx_q_show(file, data, true);
>  }
>  DEFINE_SHOW_ATTRIBUTE(nfp_xdp_q);
>  
> -- 
> 2.39.5
> 

[PATCH 07/11] debugfs: split short and full proxy wrappers, kill debugfs_real_fops()

[Al Viro]

All users outside of fs/debugfs/file.c are gone, in there we can just
fully split the wrappers for full and short cases and be done with that.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/debugfs/file.c       | 87 ++++++++++++++++++-----------------------
 include/linux/debugfs.h |  2 -
 2 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 69e9ddcb113d..77784091a10f 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -53,23 +53,6 @@ const void *debugfs_get_aux(const struct file *file)
 }
 EXPORT_SYMBOL_GPL(debugfs_get_aux);
 
-const struct file_operations *debugfs_real_fops(const struct file *filp)
-{
-	struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
-
-	if (!fsd) {
-		/*
-		 * Urgh, we've been called w/o a protecting
-		 * debugfs_file_get().
-		 */
-		WARN_ON(1);
-		return NULL;
-	}
-
-	return fsd->real_fops;
-}
-EXPORT_SYMBOL_GPL(debugfs_real_fops);
-
 enum dbgfs_get_mode {
 	DBGFS_GET_ALREADY,
 	DBGFS_GET_REGULAR,
@@ -302,15 +285,13 @@ static int debugfs_locked_down(struct inode *inode,
 static int open_proxy_open(struct inode *inode, struct file *filp)
 {
 	struct dentry *dentry = F_DENTRY(filp);
-	const struct file_operations *real_fops = NULL;
+	const struct file_operations *real_fops = DEBUGFS_I(inode)->real_fops;
 	int r;
 
 	r = __debugfs_file_get(dentry, DBGFS_GET_REGULAR);
 	if (r)
 		return r == -EIO ? -ENOENT : r;
 
-	real_fops = debugfs_real_fops(filp);
-
 	r = debugfs_locked_down(inode, filp, real_fops);
 	if (r)
 		goto out;
@@ -352,7 +333,6 @@ static ret_type full_proxy_ ## name(proto)				\
 {									\
 	struct dentry *dentry = F_DENTRY(filp);				\
 	struct debugfs_fsdata *fsd = dentry->d_fsdata;			\
-	const struct file_operations *real_fops;			\
 	ret_type r;							\
 									\
 	if (!(fsd->methods & bit))					\
@@ -360,14 +340,13 @@ static ret_type full_proxy_ ## name(proto)				\
 	r = debugfs_file_get(dentry);					\
 	if (unlikely(r))						\
 		return r;						\
-	real_fops = debugfs_real_fops(filp);				\
-	r = real_fops->name(args);					\
+	r = fsd->real_fops->name(args);					\
 	debugfs_file_put(dentry);					\
 	return r;							\
 }
 
-#define FULL_PROXY_FUNC_BOTH(name, ret_type, filp, proto, args, bit, ret)	\
-static ret_type full_proxy_ ## name(proto)				\
+#define SHORT_PROXY_FUNC(name, ret_type, filp, proto, args, bit, ret)	\
+static ret_type short_proxy_ ## name(proto)				\
 {									\
 	struct dentry *dentry = F_DENTRY(filp);				\
 	struct debugfs_fsdata *fsd = dentry->d_fsdata;			\
@@ -378,27 +357,38 @@ static ret_type full_proxy_ ## name(proto)				\
 	r = debugfs_file_get(dentry);					\
 	if (unlikely(r))						\
 		return r;						\
-	if (fsd->real_fops)						\
-		r = fsd->real_fops->name(args);				\
-	else								\
-		r = fsd->short_fops->name(args);			\
+	r = fsd->short_fops->name(args);				\
 	debugfs_file_put(dentry);					\
 	return r;							\
 }
 
-FULL_PROXY_FUNC_BOTH(llseek, loff_t, filp,
-		     PROTO(struct file *filp, loff_t offset, int whence),
-		     ARGS(filp, offset, whence), HAS_LSEEK, -ESPIPE);
+SHORT_PROXY_FUNC(llseek, loff_t, filp,
+		PROTO(struct file *filp, loff_t offset, int whence),
+		ARGS(filp, offset, whence), HAS_LSEEK, -ESPIPE);
 
-FULL_PROXY_FUNC_BOTH(read, ssize_t, filp,
-		     PROTO(struct file *filp, char __user *buf, size_t size,
-			   loff_t *ppos),
-		     ARGS(filp, buf, size, ppos), HAS_READ, -EINVAL);
+FULL_PROXY_FUNC(llseek, loff_t, filp,
+		PROTO(struct file *filp, loff_t offset, int whence),
+		ARGS(filp, offset, whence), HAS_LSEEK, -ESPIPE);
 
-FULL_PROXY_FUNC_BOTH(write, ssize_t, filp,
-		     PROTO(struct file *filp, const char __user *buf,
-			   size_t size, loff_t *ppos),
-		     ARGS(filp, buf, size, ppos), HAS_WRITE, -EINVAL);
+SHORT_PROXY_FUNC(read, ssize_t, filp,
+		PROTO(struct file *filp, char __user *buf, size_t size,
+			loff_t *ppos),
+		ARGS(filp, buf, size, ppos), HAS_READ, -EINVAL);
+
+FULL_PROXY_FUNC(read, ssize_t, filp,
+		PROTO(struct file *filp, char __user *buf, size_t size,
+			loff_t *ppos),
+		ARGS(filp, buf, size, ppos), HAS_READ, -EINVAL);
+
+SHORT_PROXY_FUNC(write, ssize_t, filp,
+		PROTO(struct file *filp, const char __user *buf,
+			size_t size, loff_t *ppos),
+		ARGS(filp, buf, size, ppos), HAS_WRITE, -EINVAL);
+
+FULL_PROXY_FUNC(write, ssize_t, filp,
+		PROTO(struct file *filp, const char __user *buf,
+			size_t size, loff_t *ppos),
+		ARGS(filp, buf, size, ppos), HAS_WRITE, -EINVAL);
 
 FULL_PROXY_FUNC(unlocked_ioctl, long, filp,
 		PROTO(struct file *filp, unsigned int cmd, unsigned long arg),
@@ -410,22 +400,21 @@ static __poll_t full_proxy_poll(struct file *filp,
 	struct dentry *dentry = F_DENTRY(filp);
 	struct debugfs_fsdata *fsd = dentry->d_fsdata;
 	__poll_t r = 0;
-	const struct file_operations *real_fops;
 
 	if (!(fsd->methods & HAS_POLL))
 		return DEFAULT_POLLMASK;
 	if (debugfs_file_get(dentry))
 		return EPOLLHUP;
 
-	real_fops = debugfs_real_fops(filp);
-	r = real_fops->poll(filp, wait);
+	r = fsd->real_fops->poll(filp, wait);
 	debugfs_file_put(dentry);
 	return r;
 }
 
-static int full_proxy_release(struct inode *inode, struct file *filp)
+static int full_proxy_release(struct inode *inode, struct file *file)
 {
-	const struct file_operations *real_fops = debugfs_real_fops(filp);
+	struct debugfs_fsdata *fsd = F_DENTRY(file)->d_fsdata;
+	const struct file_operations *real_fops = fsd->real_fops;
 	int r = 0;
 
 	/*
@@ -435,7 +424,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
 	 * ->i_private is still being meaningful here.
 	 */
 	if (real_fops->release)
-		r = real_fops->release(inode, filp);
+		r = real_fops->release(inode, file);
 
 	fops_put(real_fops);
 	return r;
@@ -517,9 +506,9 @@ static int full_proxy_open_short(struct inode *inode, struct file *filp)
 
 const struct file_operations debugfs_full_short_proxy_file_operations = {
 	.open = full_proxy_open_short,
-	.llseek = full_proxy_llseek,
-	.read = full_proxy_read,
-	.write = full_proxy_write,
+	.llseek = short_proxy_llseek,
+	.read = short_proxy_read,
+	.write = short_proxy_write,
 };
 
 ssize_t debugfs_attr_read(struct file *file, char __user *buf,
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index fa2568b4380d..a420152105d0 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -162,7 +162,6 @@ void debugfs_remove(struct dentry *dentry);
 
 void debugfs_lookup_and_remove(const char *name, struct dentry *parent);
 
-const struct file_operations *debugfs_real_fops(const struct file *filp);
 const void *debugfs_get_aux(const struct file *file);
 
 int debugfs_file_get(struct dentry *dentry);
@@ -329,7 +328,6 @@ static inline void debugfs_lookup_and_remove(const char *name,
 					     struct dentry *parent)
 { }
 
-const struct file_operations *debugfs_real_fops(const struct file *filp);
 void *debugfs_get_aux(const struct file *file);
 
 static inline int debugfs_file_get(struct dentry *dentry)
-- 
2.39.5

[PATCH 08/11] fix tt_command_write()

[Al Viro]

1) unbalanced debugfs_file_get().  Not needed in the first place -
file_operations are accessed only via debugfs_create_file(), so
debugfs wrappers will take care of that itself.

2) kmalloc() for a buffer used only for duration of a function is not
a problem, but for a buffer no longer than 16 bytes?

3) strstr() is for finding substrings; for finding a character there's
strchr().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/thermal/testing/command.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/testing/command.c b/drivers/thermal/testing/command.c
index ba11d70e8021..1159ecea57e7 100644
--- a/drivers/thermal/testing/command.c
+++ b/drivers/thermal/testing/command.c
@@ -139,31 +139,21 @@ static int tt_command_exec(int index, const char *arg)
 	return ret;
 }
 
-static ssize_t tt_command_process(struct dentry *dentry, const char __user *user_buf,
-				  size_t count)
+static ssize_t tt_command_process(char *s)
 {
-	char *buf __free(kfree);
 	char *arg;
 	int i;
 
-	buf = kmalloc(count + 1, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	strim(s);
 
-	if (copy_from_user(buf, user_buf, count))
-		return -EFAULT;
-
-	buf[count] = '\0';
-	strim(buf);
-
-	arg = strstr(buf, ":");
+	arg = strchr(s, ':');
 	if (arg) {
 		*arg = '\0';
 		arg++;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(tt_command_strings); i++) {
-		if (!strcmp(buf, tt_command_strings[i]))
+		if (!strcmp(s, tt_command_strings[i]))
 			return tt_command_exec(i, arg);
 	}
 
@@ -173,20 +163,20 @@ static ssize_t tt_command_process(struct dentry *dentry, const char __user *user
 static ssize_t tt_command_write(struct file *file, const char __user *user_buf,
 				size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = file->f_path.dentry;
+	char buf[TT_COMMAND_SIZE];
 	ssize_t ret;
 
 	if (*ppos)
 		return -EINVAL;
 
-	if (count + 1 > TT_COMMAND_SIZE)
+	if (count > TT_COMMAND_SIZE - 1)
 		return -E2BIG;
 
-	ret = debugfs_file_get(dentry);
-	if (unlikely(ret))
-		return ret;
+	if (copy_from_user(buf, user_buf, count))
+		return -EFAULT;
+	buf[count] = '\0';
 
-	ret = tt_command_process(dentry, user_buf, count);
+	ret = tt_command_process(buf);
 	if (ret)
 		return ret;
 
-- 
2.39.5

Re: [PATCH 08/11] fix tt_command_write()

[Rafael J. Wysocki]

On Wed, Jul 2, 2025 at 11:25 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> 1) unbalanced debugfs_file_get().  Not needed in the first place -
> file_operations are accessed only via debugfs_create_file(), so
> debugfs wrappers will take care of that itself.
>
> 2) kmalloc() for a buffer used only for duration of a function is not
> a problem, but for a buffer no longer than 16 bytes?
>
> 3) strstr() is for finding substrings; for finding a character there's
> strchr().
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

Or do you want me to apply this?

> ---
>  drivers/thermal/testing/command.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/thermal/testing/command.c b/drivers/thermal/testing/command.c
> index ba11d70e8021..1159ecea57e7 100644
> --- a/drivers/thermal/testing/command.c
> +++ b/drivers/thermal/testing/command.c
> @@ -139,31 +139,21 @@ static int tt_command_exec(int index, const char *arg)
>         return ret;
>  }
>
> -static ssize_t tt_command_process(struct dentry *dentry, const char __user *user_buf,
> -                                 size_t count)
> +static ssize_t tt_command_process(char *s)
>  {
> -       char *buf __free(kfree);
>         char *arg;
>         int i;
>
> -       buf = kmalloc(count + 1, GFP_KERNEL);
> -       if (!buf)
> -               return -ENOMEM;
> +       strim(s);
>
> -       if (copy_from_user(buf, user_buf, count))
> -               return -EFAULT;
> -
> -       buf[count] = '\0';
> -       strim(buf);
> -
> -       arg = strstr(buf, ":");
> +       arg = strchr(s, ':');
>         if (arg) {
>                 *arg = '\0';
>                 arg++;
>         }
>
>         for (i = 0; i < ARRAY_SIZE(tt_command_strings); i++) {
> -               if (!strcmp(buf, tt_command_strings[i]))
> +               if (!strcmp(s, tt_command_strings[i]))
>                         return tt_command_exec(i, arg);
>         }
>
> @@ -173,20 +163,20 @@ static ssize_t tt_command_process(struct dentry *dentry, const char __user *user
>  static ssize_t tt_command_write(struct file *file, const char __user *user_buf,
>                                 size_t count, loff_t *ppos)
>  {
> -       struct dentry *dentry = file->f_path.dentry;
> +       char buf[TT_COMMAND_SIZE];
>         ssize_t ret;
>
>         if (*ppos)
>                 return -EINVAL;
>
> -       if (count + 1 > TT_COMMAND_SIZE)
> +       if (count > TT_COMMAND_SIZE - 1)
>                 return -E2BIG;
>
> -       ret = debugfs_file_get(dentry);
> -       if (unlikely(ret))
> -               return ret;
> +       if (copy_from_user(buf, user_buf, count))
> +               return -EFAULT;
> +       buf[count] = '\0';
>
> -       ret = tt_command_process(dentry, user_buf, count);
> +       ret = tt_command_process(buf);
>         if (ret)
>                 return ret;
>
> --
> 2.39.5
>
>

[PATCH 09/11] debugfs_get_aux(): allow storing non-const void *

[Al Viro]

typechecking is up to users, anyway...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/staging/greybus/camera.c | 2 +-
 fs/debugfs/file.c                | 2 +-
 fs/debugfs/inode.c               | 2 +-
 fs/debugfs/internal.h            | 2 +-
 include/linux/debugfs.h          | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index ec9fddfc0b14..5ac19c0055d9 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -1128,7 +1128,7 @@ static ssize_t gb_camera_debugfs_write(struct file *file,
 
 static int gb_camera_debugfs_open(struct inode *inode, struct file *file)
 {
-	file->private_data = (void *)debugfs_get_aux(file);
+	file->private_data = debugfs_get_aux(file);
 	return 0;
 }
 
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 77784091a10f..3ec3324c2060 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -47,7 +47,7 @@ const struct file_operations debugfs_noop_file_operations = {
 
 #define F_DENTRY(filp) ((filp)->f_path.dentry)
 
-const void *debugfs_get_aux(const struct file *file)
+void *debugfs_get_aux(const struct file *file)
 {
 	return DEBUGFS_I(file_inode(file))->aux;
 }
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 30c4944e1862..43e5d1bf1f32 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -459,7 +459,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 		proxy_fops = &debugfs_noop_file_operations;
 	inode->i_fop = proxy_fops;
 	DEBUGFS_I(inode)->raw = real_fops;
-	DEBUGFS_I(inode)->aux = aux;
+	DEBUGFS_I(inode)->aux = (void *)aux;
 
 	d_instantiate(dentry, inode);
 	fsnotify_create(d_inode(dentry->d_parent), dentry);
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 93483fe84425..427987f81571 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -19,7 +19,7 @@ struct debugfs_inode_info {
 		const struct debugfs_short_fops *short_fops;
 		debugfs_automount_t automount;
 	};
-	const void *aux;
+	void *aux;
 };
 
 static inline struct debugfs_inode_info *DEBUGFS_I(struct inode *inode)
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index a420152105d0..7cecda29447e 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -162,7 +162,7 @@ void debugfs_remove(struct dentry *dentry);
 
 void debugfs_lookup_and_remove(const char *name, struct dentry *parent);
 
-const void *debugfs_get_aux(const struct file *file);
+void *debugfs_get_aux(const struct file *file);
 
 int debugfs_file_get(struct dentry *dentry);
 void debugfs_file_put(struct dentry *dentry);
-- 
2.39.5

[PATCH 10/11] blk-mq-debugfs: use debugfs_get_aux()

[Al Viro]

instead of manually stashing the data pointer into parent directory inode's
->i_private, just pass it to debugfs_create_file_aux() so that it can
be extracted without that insane chasing through ->d_parent.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 block/blk-mq-debugfs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 29b3540dd180..7ed3e71f2fc0 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -521,7 +521,7 @@ CTX_RQ_SEQ_OPS(poll, HCTX_TYPE_POLL);
 static int blk_mq_debugfs_show(struct seq_file *m, void *v)
 {
 	const struct blk_mq_debugfs_attr *attr = m->private;
-	void *data = d_inode(m->file->f_path.dentry->d_parent)->i_private;
+	void *data = debugfs_get_aux(m->file);
 
 	return attr->show(data, m);
 }
@@ -531,7 +531,7 @@ static ssize_t blk_mq_debugfs_write(struct file *file, const char __user *buf,
 {
 	struct seq_file *m = file->private_data;
 	const struct blk_mq_debugfs_attr *attr = m->private;
-	void *data = d_inode(file->f_path.dentry->d_parent)->i_private;
+	void *data = debugfs_get_aux(file);
 
 	/*
 	 * Attributes that only implement .seq_ops are read-only and 'attr' is
@@ -546,7 +546,7 @@ static ssize_t blk_mq_debugfs_write(struct file *file, const char __user *buf,
 static int blk_mq_debugfs_open(struct inode *inode, struct file *file)
 {
 	const struct blk_mq_debugfs_attr *attr = inode->i_private;
-	void *data = d_inode(file->f_path.dentry->d_parent)->i_private;
+	void *data = debugfs_get_aux(file);
 	struct seq_file *m;
 	int ret;
 
@@ -612,11 +612,9 @@ static void debugfs_create_files(struct dentry *parent, void *data,
 	if (IS_ERR_OR_NULL(parent))
 		return;
 
-	d_inode(parent)->i_private = data;
-
 	for (; attr->name; attr++)
-		debugfs_create_file(attr->name, attr->mode, parent,
-				    (void *)attr, &blk_mq_debugfs_fops);
+		debugfs_create_file_aux(attr->name, attr->mode, parent,
+				    (void *)attr, data, &blk_mq_debugfs_fops);
 }
 
 void blk_mq_debugfs_register(struct request_queue *q)
-- 
2.39.5

[PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Al Viro]

If you want a home-grown switch, at least use enum for selector...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/scsi/lpfc/lpfc_debugfs.c | 99 ++++++++++++++++----------------
 1 file changed, 51 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 3fd1aa5cc78c..55ff030ca6cd 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -2371,36 +2371,48 @@ lpfc_debugfs_dumpHostSlim_open(struct inode *inode, struct file *file)
 	return rc;
 }
 
+enum {
+	writeGuard = 1,
+	writeApp,
+	writeRef,
+	readGuard,
+	readApp,
+	readRef,
+	InjErrLBA,
+	InjErrNPortID,
+	InjErrWWPN,
+};
+
 static ssize_t
 lpfc_debugfs_dif_err_read(struct file *file, char __user *buf,
 	size_t nbytes, loff_t *ppos)
 {
-	struct dentry *dent = file->f_path.dentry;
 	struct lpfc_hba *phba = file->private_data;
+	int kind = debugfs_get_aux_num(file);
 	char cbuf[32];
 	uint64_t tmp = 0;
 	int cnt = 0;
 
-	if (dent == phba->debug_writeGuard)
+	if (kind == writeGuard)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wgrd_cnt);
-	else if (dent == phba->debug_writeApp)
+	else if (kind == writeApp)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wapp_cnt);
-	else if (dent == phba->debug_writeRef)
+	else if (kind == writeRef)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wref_cnt);
-	else if (dent == phba->debug_readGuard)
+	else if (kind == readGuard)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rgrd_cnt);
-	else if (dent == phba->debug_readApp)
+	else if (kind == readApp)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rapp_cnt);
-	else if (dent == phba->debug_readRef)
+	else if (kind == readRef)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rref_cnt);
-	else if (dent == phba->debug_InjErrNPortID)
+	else if (kind == InjErrNPortID)
 		cnt = scnprintf(cbuf, 32, "0x%06x\n",
 				phba->lpfc_injerr_nportid);
-	else if (dent == phba->debug_InjErrWWPN) {
+	else if (kind == InjErrWWPN) {
 		memcpy(&tmp, &phba->lpfc_injerr_wwpn, sizeof(struct lpfc_name));
 		tmp = cpu_to_be64(tmp);
 		cnt = scnprintf(cbuf, 32, "0x%016llx\n", tmp);
-	} else if (dent == phba->debug_InjErrLBA) {
+	} else if (kind == InjErrLBA) {
 		if (phba->lpfc_injerr_lba == (sector_t)(-1))
 			cnt = scnprintf(cbuf, 32, "off\n");
 		else
@@ -2417,8 +2429,8 @@ static ssize_t
 lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	size_t nbytes, loff_t *ppos)
 {
-	struct dentry *dent = file->f_path.dentry;
 	struct lpfc_hba *phba = file->private_data;
+	int kind = debugfs_get_aux_num(file);
 	char dstbuf[33];
 	uint64_t tmp = 0;
 	int size;
@@ -2428,7 +2440,7 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	if (copy_from_user(dstbuf, buf, size))
 		return -EFAULT;
 
-	if (dent == phba->debug_InjErrLBA) {
+	if (kind == InjErrLBA) {
 		if ((dstbuf[0] == 'o') && (dstbuf[1] == 'f') &&
 		    (dstbuf[2] == 'f'))
 			tmp = (uint64_t)(-1);
@@ -2437,23 +2449,23 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	if ((tmp == 0) && (kstrtoull(dstbuf, 0, &tmp)))
 		return -EINVAL;
 
-	if (dent == phba->debug_writeGuard)
+	if (kind == writeGuard)
 		phba->lpfc_injerr_wgrd_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_writeApp)
+	else if (kind == writeApp)
 		phba->lpfc_injerr_wapp_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_writeRef)
+	else if (kind == writeRef)
 		phba->lpfc_injerr_wref_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readGuard)
+	else if (kind == readGuard)
 		phba->lpfc_injerr_rgrd_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readApp)
+	else if (kind == readApp)
 		phba->lpfc_injerr_rapp_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readRef)
+	else if (kind == readRef)
 		phba->lpfc_injerr_rref_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_InjErrLBA)
+	else if (kind == InjErrLBA)
 		phba->lpfc_injerr_lba = (sector_t)tmp;
-	else if (dent == phba->debug_InjErrNPortID)
+	else if (kind == InjErrNPortID)
 		phba->lpfc_injerr_nportid = (uint32_t)(tmp & Mask_DID);
-	else if (dent == phba->debug_InjErrWWPN) {
+	else if (kind == InjErrWWPN) {
 		tmp = cpu_to_be64(tmp);
 		memcpy(&phba->lpfc_injerr_wwpn, &tmp, sizeof(struct lpfc_name));
 	} else
@@ -6160,60 +6172,51 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 			phba->debug_dumpHostSlim = NULL;
 
 		/* Setup DIF Error Injections */
-		snprintf(name, sizeof(name), "InjErrLBA");
 		phba->debug_InjErrLBA =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrLBA", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrLBA, &lpfc_debugfs_op_dif_err);
 		phba->lpfc_injerr_lba = LPFC_INJERR_LBA_OFF;
 
-		snprintf(name, sizeof(name), "InjErrNPortID");
 		phba->debug_InjErrNPortID =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrNPortID", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrNPortID, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "InjErrWWPN");
 		phba->debug_InjErrWWPN =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrWWPN", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrWWPN, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeGuardInjErr");
 		phba->debug_writeGuard =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeGuardInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeGuard, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeAppInjErr");
 		phba->debug_writeApp =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeAppInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeApp, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeRefInjErr");
 		phba->debug_writeRef =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeRefInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeRef, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readGuardInjErr");
 		phba->debug_readGuard =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readGuardInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readGuard, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readAppInjErr");
 		phba->debug_readApp =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readAppInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readApp, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readRefInjErr");
 		phba->debug_readRef =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readRefInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readRef, &lpfc_debugfs_op_dif_err);
 
 		/* Setup slow ring trace */
 		if (lpfc_debugfs_max_slow_ring_trc) {
-- 
2.39.5

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Justin Tee]

Hi Al,

I’m good with the use of enum.  For the if and else if blocks, would
it be possible to help us out and convert those to switch case
statements?

For example,

switch (kind) {
case writeGuard :
    cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wgrd_cnt);
    break;
case writeApp:
    cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wapp_cnt);
    break;
…
default:
    lpfc_printf_log(...);
}

Thanks,
Justin

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Al Viro]

On Thu, Jul 03, 2025 at 01:37:53PM -0700, Justin Tee wrote:
> Hi Al,
> 
> I’m good with the use of enum.  For the if and else if blocks, would
> it be possible to help us out and convert those to switch case
> statements?
> 
> For example,
> 
> switch (kind) {
> case writeGuard :
>     cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wgrd_cnt);
>     break;
> case writeApp:
>     cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wapp_cnt);
>     break;
> …
> default:
>     lpfc_printf_log(...);
> }

Sure, but I'd rather do that as a followup.  Speaking of other fun followups
in the same area: no point storing most of those dentries; debugfs_remove()
takes the entire subtree out, no need to remove them one-by-one and that
was the only use they were left...  Something along the lines of
diff below:

diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index fe4fb67eb50c..230f0377c1db 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -744,12 +744,6 @@ struct lpfc_vport {
 	struct lpfc_vmid_priority_info vmid_priority;
 
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
-	struct dentry *debug_disc_trc;
-	struct dentry *debug_nodelist;
-	struct dentry *debug_nvmestat;
-	struct dentry *debug_scsistat;
-	struct dentry *debug_ioktime;
-	struct dentry *debug_hdwqstat;
 	struct dentry *vport_debugfs_root;
 	struct lpfc_debugfs_trc *disc_trc;
 	atomic_t disc_trc_cnt;
@@ -1349,29 +1343,8 @@ struct lpfc_hba {
 #ifdef CONFIG_SCSI_LPFC_DEBUG_FS
 	struct dentry *hba_debugfs_root;
 	atomic_t debugfs_vport_count;
-	struct dentry *debug_multixri_pools;
-	struct dentry *debug_hbqinfo;
-	struct dentry *debug_dumpHostSlim;
-	struct dentry *debug_dumpHBASlim;
-	struct dentry *debug_InjErrLBA;  /* LBA to inject errors at */
-	struct dentry *debug_InjErrNPortID;  /* NPortID to inject errors at */
-	struct dentry *debug_InjErrWWPN;  /* WWPN to inject errors at */
-	struct dentry *debug_writeGuard; /* inject write guard_tag errors */
-	struct dentry *debug_writeApp;   /* inject write app_tag errors */
-	struct dentry *debug_writeRef;   /* inject write ref_tag errors */
-	struct dentry *debug_readGuard;  /* inject read guard_tag errors */
-	struct dentry *debug_readApp;    /* inject read app_tag errors */
-	struct dentry *debug_readRef;    /* inject read ref_tag errors */
-
-	struct dentry *debug_nvmeio_trc;
+
 	struct lpfc_debugfs_nvmeio_trc *nvmeio_trc;
-	struct dentry *debug_hdwqinfo;
-#ifdef LPFC_HDWQ_LOCK_STAT
-	struct dentry *debug_lockstat;
-#endif
-	struct dentry *debug_cgn_buffer;
-	struct dentry *debug_rx_monitor;
-	struct dentry *debug_ras_log;
 	atomic_t nvmeio_trc_cnt;
 	uint32_t nvmeio_trc_size;
 	uint32_t nvmeio_trc_output_idx;
@@ -1388,7 +1361,6 @@ struct lpfc_hba {
 	sector_t lpfc_injerr_lba;
 #define LPFC_INJERR_LBA_OFF	(sector_t)(-1)
 
-	struct dentry *debug_slow_ring_trc;
 	struct lpfc_debugfs_trc *slow_ring_trc;
 	atomic_t slow_ring_trc_cnt;
 	/* iDiag debugfs sub-directory */
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 55ff030ca6cd..51f74a0735dc 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -6055,6 +6055,7 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 	char name[64];
 	uint32_t num, i;
 	bool pport_setup = false;
+	struct dentry *dentry;
 
 	if (!lpfc_debugfs_enable)
 		return;
@@ -6077,25 +6078,23 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 		atomic_set(&phba->debugfs_vport_count, 0);
 
 		/* Multi-XRI pools */
-		snprintf(name, sizeof(name), "multixripools");
-		phba->debug_multixri_pools =
-			debugfs_create_file(name, S_IFREG | 0644,
+		dentry =
+			debugfs_create_file("multixripools", S_IFREG | 0644,
 					    phba->hba_debugfs_root,
 					    phba,
 					    &lpfc_debugfs_op_multixripools);
-		if (IS_ERR(phba->debug_multixri_pools)) {
+		if (IS_ERR(dentry)) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 					 "0527 Cannot create debugfs multixripools\n");
 			goto debug_failed;
 		}
 
 		/* Congestion Info Buffer */
-		scnprintf(name, sizeof(name), "cgn_buffer");
-		phba->debug_cgn_buffer =
-			debugfs_create_file(name, S_IFREG | 0644,
+		dentry =
+			debugfs_create_file("cgn_buffer", S_IFREG | 0644,
 					    phba->hba_debugfs_root,
 					    phba, &lpfc_cgn_buffer_op);
-		if (IS_ERR(phba->debug_cgn_buffer)) {
+		if (IS_ERR(dentry)) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 					 "6527 Cannot create debugfs "
 					 "cgn_buffer\n");
@@ -6103,12 +6102,11 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 		}
 
 		/* RX Monitor */
-		scnprintf(name, sizeof(name), "rx_monitor");
-		phba->debug_rx_monitor =
-			debugfs_create_file(name, S_IFREG | 0644,
+		dentry =
+			debugfs_create_file("rx_monitor", S_IFREG | 0644,
 					    phba->hba_debugfs_root,
 					    phba, &lpfc_rx_monitor_op);
-		if (IS_ERR(phba->debug_rx_monitor)) {
+		if (IS_ERR(dentry)) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 					 "6528 Cannot create debugfs "
 					 "rx_monitor\n");
@@ -6116,12 +6114,11 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 		}
 
 		/* RAS log */
-		snprintf(name, sizeof(name), "ras_log");
-		phba->debug_ras_log =
-			debugfs_create_file(name, 0644,
+		dentry =
+			debugfs_create_file("ras_log", 0644,
 					    phba->hba_debugfs_root,
 					    phba, &lpfc_debugfs_ras_log);
-		if (IS_ERR(phba->debug_ras_log)) {
+		if (IS_ERR(dentry)) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 					 "6148 Cannot create debugfs"
 					 " ras_log\n");
@@ -6129,92 +6126,74 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 		}
 
 		/* Setup hbqinfo */
-		snprintf(name, sizeof(name), "hbqinfo");
-		phba->debug_hbqinfo =
-			debugfs_create_file(name, S_IFREG | 0644,
-					    phba->hba_debugfs_root,
-					    phba, &lpfc_debugfs_op_hbqinfo);
+		debugfs_create_file("hbqinfo", S_IFREG | 0644,
+				    phba->hba_debugfs_root,
+				    phba, &lpfc_debugfs_op_hbqinfo);
 
 #ifdef LPFC_HDWQ_LOCK_STAT
 		/* Setup lockstat */
-		snprintf(name, sizeof(name), "lockstat");
-		phba->debug_lockstat =
-			debugfs_create_file(name, S_IFREG | 0644,
+		dentry =
+			debugfs_create_file("lockstat", S_IFREG | 0644,
 					    phba->hba_debugfs_root,
 					    phba, &lpfc_debugfs_op_lockstat);
-		if (IS_ERR(phba->debug_lockstat)) {
+		if (IS_ERR(dentry)) {
 			lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 					 "4610 Can't create debugfs lockstat\n");
 			goto debug_failed;
 		}
 #endif
 
-		/* Setup dumpHBASlim */
 		if (phba->sli_rev < LPFC_SLI_REV4) {
-			snprintf(name, sizeof(name), "dumpHBASlim");
-			phba->debug_dumpHBASlim =
-				debugfs_create_file(name,
+			/* Setup dumpHBASlim */
+			debugfs_create_file("dumpHBASlim",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					phba->hba_debugfs_root,
 					phba, &lpfc_debugfs_op_dumpHBASlim);
-		} else
-			phba->debug_dumpHBASlim = NULL;
+		}
 
-		/* Setup dumpHostSlim */
 		if (phba->sli_rev < LPFC_SLI_REV4) {
-			snprintf(name, sizeof(name), "dumpHostSlim");
-			phba->debug_dumpHostSlim =
-				debugfs_create_file(name,
+			/* Setup dumpHostSlim */
+			debugfs_create_file("dumpHostSlim",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					phba->hba_debugfs_root,
 					phba, &lpfc_debugfs_op_dumpHostSlim);
-		} else
-			phba->debug_dumpHostSlim = NULL;
+		}
 
 		/* Setup DIF Error Injections */
-		phba->debug_InjErrLBA =
-			debugfs_create_file_aux_num("InjErrLBA", 0644,
+		debugfs_create_file_aux_num("InjErrLBA", 0644,
 			phba->hba_debugfs_root,
 			phba, InjErrLBA, &lpfc_debugfs_op_dif_err);
 		phba->lpfc_injerr_lba = LPFC_INJERR_LBA_OFF;
 
-		phba->debug_InjErrNPortID =
-			debugfs_create_file_aux_num("InjErrNPortID", 0644,
+		debugfs_create_file_aux_num("InjErrNPortID", 0644,
 			phba->hba_debugfs_root,
 			phba, InjErrNPortID, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_InjErrWWPN =
-			debugfs_create_file_aux_num("InjErrWWPN", 0644,
+		debugfs_create_file_aux_num("InjErrWWPN", 0644,
 			phba->hba_debugfs_root,
 			phba, InjErrWWPN, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_writeGuard =
-			debugfs_create_file_aux_num("writeGuardInjErr", 0644,
+		debugfs_create_file_aux_num("writeGuardInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, writeGuard, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_writeApp =
-			debugfs_create_file_aux_num("writeAppInjErr", 0644,
+		debugfs_create_file_aux_num("writeAppInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, writeApp, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_writeRef =
-			debugfs_create_file_aux_num("writeRefInjErr", 0644,
+		debugfs_create_file_aux_num("writeRefInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, writeRef, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_readGuard =
-			debugfs_create_file_aux_num("readGuardInjErr", 0644,
+		debugfs_create_file_aux_num("readGuardInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, readGuard, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_readApp =
-			debugfs_create_file_aux_num("readAppInjErr", 0644,
+		debugfs_create_file_aux_num("readAppInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, readApp, &lpfc_debugfs_op_dif_err);
 
-		phba->debug_readRef =
-			debugfs_create_file_aux_num("readRefInjErr", 0644,
+		debugfs_create_file_aux_num("readRefInjErr", 0644,
 			phba->hba_debugfs_root,
 			phba, readRef, &lpfc_debugfs_op_dif_err);
 
@@ -6235,9 +6214,7 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 			}
 		}
 
-		snprintf(name, sizeof(name), "slow_ring_trace");
-		phba->debug_slow_ring_trc =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+		debugfs_create_file("slow_ring_trace", S_IFREG|S_IRUGO|S_IWUSR,
 				 phba->hba_debugfs_root,
 				 phba, &lpfc_debugfs_op_slow_ring_trc);
 		if (!phba->slow_ring_trc) {
@@ -6254,9 +6231,7 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 			atomic_set(&phba->slow_ring_trc_cnt, 0);
 		}
 
-		snprintf(name, sizeof(name), "nvmeio_trc");
-		phba->debug_nvmeio_trc =
-			debugfs_create_file(name, 0644,
+		debugfs_create_file("nvmeio_trc", 0644,
 					    phba->hba_debugfs_root,
 					    phba, &lpfc_debugfs_op_nvmeio_trc);
 
@@ -6337,48 +6312,38 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 	}
 	atomic_set(&vport->disc_trc_cnt, 0);
 
-	snprintf(name, sizeof(name), "discovery_trace");
-	vport->debug_disc_trc =
-		debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+	debugfs_create_file("discovery_trace", S_IFREG|S_IRUGO|S_IWUSR,
 				 vport->vport_debugfs_root,
 				 vport, &lpfc_debugfs_op_disc_trc);
-	snprintf(name, sizeof(name), "nodelist");
-	vport->debug_nodelist =
-		debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+	debugfs_create_file("nodelist", S_IFREG|S_IRUGO|S_IWUSR,
 				 vport->vport_debugfs_root,
 				 vport, &lpfc_debugfs_op_nodelist);
 
-	snprintf(name, sizeof(name), "nvmestat");
-	vport->debug_nvmestat =
-		debugfs_create_file(name, 0644,
+	debugfs_create_file("nvmestat", 0644,
 				    vport->vport_debugfs_root,
 				    vport, &lpfc_debugfs_op_nvmestat);
 
-	snprintf(name, sizeof(name), "scsistat");
-	vport->debug_scsistat =
-		debugfs_create_file(name, 0644,
+	dentry =
+		debugfs_create_file("scsistat", 0644,
 				    vport->vport_debugfs_root,
 				    vport, &lpfc_debugfs_op_scsistat);
-	if (IS_ERR(vport->debug_scsistat)) {
+	if (IS_ERR(dentry)) {
 		lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 				 "4611 Cannot create debugfs scsistat\n");
 		goto debug_failed;
 	}
 
-	snprintf(name, sizeof(name), "ioktime");
-	vport->debug_ioktime =
-		debugfs_create_file(name, 0644,
+	dentry =
+		debugfs_create_file("ioktime", 0644,
 				    vport->vport_debugfs_root,
 				    vport, &lpfc_debugfs_op_ioktime);
-	if (IS_ERR(vport->debug_ioktime)) {
+	if (IS_ERR(dentry)) {
 		lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
 				 "0815 Cannot create debugfs ioktime\n");
 		goto debug_failed;
 	}
 
-	snprintf(name, sizeof(name), "hdwqstat");
-	vport->debug_hdwqstat =
-		debugfs_create_file(name, 0644,
+	debugfs_create_file("hdwqstat", 0644,
 				    vport->vport_debugfs_root,
 				    vport, &lpfc_debugfs_op_hdwqstat);
 
@@ -6499,24 +6464,6 @@ lpfc_debugfs_terminate(struct lpfc_vport *vport)
 	kfree(vport->disc_trc);
 	vport->disc_trc = NULL;
 
-	debugfs_remove(vport->debug_disc_trc); /* discovery_trace */
-	vport->debug_disc_trc = NULL;
-
-	debugfs_remove(vport->debug_nodelist); /* nodelist */
-	vport->debug_nodelist = NULL;
-
-	debugfs_remove(vport->debug_nvmestat); /* nvmestat */
-	vport->debug_nvmestat = NULL;
-
-	debugfs_remove(vport->debug_scsistat); /* scsistat */
-	vport->debug_scsistat = NULL;
-
-	debugfs_remove(vport->debug_ioktime); /* ioktime */
-	vport->debug_ioktime = NULL;
-
-	debugfs_remove(vport->debug_hdwqstat); /* hdwqstat */
-	vport->debug_hdwqstat = NULL;
-
 	if (vport->vport_debugfs_root) {
 		debugfs_remove(vport->vport_debugfs_root); /* vportX */
 		vport->vport_debugfs_root = NULL;
@@ -6525,68 +6472,9 @@ lpfc_debugfs_terminate(struct lpfc_vport *vport)
 
 	if (atomic_read(&phba->debugfs_vport_count) == 0) {
 
-		debugfs_remove(phba->debug_multixri_pools); /* multixripools*/
-		phba->debug_multixri_pools = NULL;
-
-		debugfs_remove(phba->debug_hbqinfo); /* hbqinfo */
-		phba->debug_hbqinfo = NULL;
-
-		debugfs_remove(phba->debug_cgn_buffer);
-		phba->debug_cgn_buffer = NULL;
-
-		debugfs_remove(phba->debug_rx_monitor);
-		phba->debug_rx_monitor = NULL;
-
-		debugfs_remove(phba->debug_ras_log);
-		phba->debug_ras_log = NULL;
-
-#ifdef LPFC_HDWQ_LOCK_STAT
-		debugfs_remove(phba->debug_lockstat); /* lockstat */
-		phba->debug_lockstat = NULL;
-#endif
-		debugfs_remove(phba->debug_dumpHBASlim); /* HBASlim */
-		phba->debug_dumpHBASlim = NULL;
-
-		debugfs_remove(phba->debug_dumpHostSlim); /* HostSlim */
-		phba->debug_dumpHostSlim = NULL;
-
-		debugfs_remove(phba->debug_InjErrLBA); /* InjErrLBA */
-		phba->debug_InjErrLBA = NULL;
-
-		debugfs_remove(phba->debug_InjErrNPortID);
-		phba->debug_InjErrNPortID = NULL;
-
-		debugfs_remove(phba->debug_InjErrWWPN); /* InjErrWWPN */
-		phba->debug_InjErrWWPN = NULL;
-
-		debugfs_remove(phba->debug_writeGuard); /* writeGuard */
-		phba->debug_writeGuard = NULL;
-
-		debugfs_remove(phba->debug_writeApp); /* writeApp */
-		phba->debug_writeApp = NULL;
-
-		debugfs_remove(phba->debug_writeRef); /* writeRef */
-		phba->debug_writeRef = NULL;
-
-		debugfs_remove(phba->debug_readGuard); /* readGuard */
-		phba->debug_readGuard = NULL;
-
-		debugfs_remove(phba->debug_readApp); /* readApp */
-		phba->debug_readApp = NULL;
-
-		debugfs_remove(phba->debug_readRef); /* readRef */
-		phba->debug_readRef = NULL;
-
 		kfree(phba->slow_ring_trc);
 		phba->slow_ring_trc = NULL;
 
-		/* slow_ring_trace */
-		debugfs_remove(phba->debug_slow_ring_trc);
-		phba->debug_slow_ring_trc = NULL;
-
-		debugfs_remove(phba->debug_nvmeio_trc);
-		phba->debug_nvmeio_trc = NULL;
-
 		kfree(phba->nvmeio_trc);
 		phba->nvmeio_trc = NULL;
 
@@ -6594,40 +6482,14 @@ lpfc_debugfs_terminate(struct lpfc_vport *vport)
 		 * iDiag release
 		 */
 		if (phba->sli_rev == LPFC_SLI_REV4) {
-			/* iDiag extAcc */
-			debugfs_remove(phba->idiag_ext_acc);
 			phba->idiag_ext_acc = NULL;
-
-			/* iDiag mbxAcc */
-			debugfs_remove(phba->idiag_mbx_acc);
 			phba->idiag_mbx_acc = NULL;
-
-			/* iDiag ctlAcc */
-			debugfs_remove(phba->idiag_ctl_acc);
 			phba->idiag_ctl_acc = NULL;
-
-			/* iDiag drbAcc */
-			debugfs_remove(phba->idiag_drb_acc);
 			phba->idiag_drb_acc = NULL;
-
-			/* iDiag queAcc */
-			debugfs_remove(phba->idiag_que_acc);
 			phba->idiag_que_acc = NULL;
-
-			/* iDiag queInfo */
-			debugfs_remove(phba->idiag_que_info);
 			phba->idiag_que_info = NULL;
-
-			/* iDiag barAcc */
-			debugfs_remove(phba->idiag_bar_acc);
 			phba->idiag_bar_acc = NULL;
-
-			/* iDiag pciCfg */
-			debugfs_remove(phba->idiag_pci_cfg);
 			phba->idiag_pci_cfg = NULL;
-
-			/* Finally remove the iDiag debugfs root */
-			debugfs_remove(phba->idiag_root);
 			phba->idiag_root = NULL;
 		}
 

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Justin Tee]

> Sure, but I'd rather do that as a followup.
Yeup, that’s fine.

> Speaking of other fun followups
> in the same area: no point storing most of those dentries; debugfs_remove()
> takes the entire subtree out, no need to remove them one-by-one and that
> was the only use they were left...  Something along the lines of
> diff below:
Very cool, thanks!  We’ll take that diff too (:

Also, may we actually move this enum declaration to lpfc_debugfs.h
instead of lpfc_debugfs.c?
enum {
        writeGuard = 1,
        writeApp,
        writeRef,
        readGuard,
        readApp,
        readRef,
        InjErrLBA,
        InjErrNPortID,
        InjErrWWPN,
};

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Al Viro]

On Fri, Jul 04, 2025 at 11:33:09AM -0700, Justin Tee wrote:
> > Sure, but I'd rather do that as a followup.
> Yeup, that’s fine.
> 
> > Speaking of other fun followups
> > in the same area: no point storing most of those dentries; debugfs_remove()
> > takes the entire subtree out, no need to remove them one-by-one and that
> > was the only use they were left...  Something along the lines of
> > diff below:
> Very cool, thanks!  We’ll take that diff too (:
> 
> Also, may we actually move this enum declaration to lpfc_debugfs.h
> instead of lpfc_debugfs.c?
> enum {
>         writeGuard = 1,
>         writeApp,
>         writeRef,
>         readGuard,
>         readApp,
>         readRef,
>         InjErrLBA,
>         InjErrNPortID,
>         InjErrWWPN,
> };

Sure, no problem...  Your driver, your preferences - it's not as if
that had any impact outside.

While we are at it, handling of debugfs_vport_count looks fishy -
it's easier to spot with aforementioned delta applied.
        if (vport->vport_debugfs_root) {
                debugfs_remove(vport->vport_debugfs_root); /* vportX */
                vport->vport_debugfs_root = NULL;
                atomic_dec(&phba->debugfs_vport_count);
        }

        if (atomic_read(&phba->debugfs_vport_count) == 0) {
		...
	}
	return;
is OK only if all updates of that thing are externally serialized.
If they are, we don't need atomic; if they are not, this separation
of decrement and test is racy.

Note that if you do *not* have external serialization, there might
be another problem if you have the last vport removal overlap
with addition of another vport.  Or, for that matter, if removal
of the last vport on the last HBA overlaps with addition of a new
HBA...

Unless something has drastically changed, binding/unbinding of
a device to driver should be serialized by drivers/base/* logics;
no idea about the vports side of things...

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Justin Tee]

> is OK only if all updates of that thing are externally serialized.
> If they are, we don't need atomic; if they are not, this separation
> of decrement and test is racy.
Agreed with this too.  Vport creation and deletion is serialized, so
we do not need to declare debugfs_vport_count as atomic.

At this point, cleaning up the lpfc_debugfs_terminate routine may be a
little more involved.  If it’s alright with you, Broadcom will take up
the responsibility to clean up the lpfc_debugfs_terminate routine
during our next driver version update, and we will of course state
your authorship in that particular clean up patch.

Regarding this thread’s enum selector patch, I can provide the signed
off by after we see that the enum declaration is moved to
lpfc_debugfs.h.

Regards,
Justin

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Al Viro]

On Mon, Jul 07, 2025 at 12:29:55PM -0700, Justin Tee wrote:
> > is OK only if all updates of that thing are externally serialized.
> > If they are, we don't need atomic; if they are not, this separation
> > of decrement and test is racy.
> Agreed with this too.  Vport creation and deletion is serialized, so
> we do not need to declare debugfs_vport_count as atomic.
> 
> At this point, cleaning up the lpfc_debugfs_terminate routine may be a
> little more involved.  If it’s alright with you, Broadcom will take up
> the responsibility to clean up the lpfc_debugfs_terminate routine
> during our next driver version update, and we will of course state
> your authorship in that particular clean up patch.
> 
> Regarding this thread’s enum selector patch, I can provide the signed
> off by after we see that the enum declaration is moved to
> lpfc_debugfs.h.

Would this do?

[PATCH v2] lpfc: don't use file->f_path.dentry for comparisons
    
If you want a home-grown switch, at least use enum for selector...
    
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 3fd1aa5cc78c..42d138ec11b4 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -2375,32 +2375,32 @@ static ssize_t
 lpfc_debugfs_dif_err_read(struct file *file, char __user *buf,
 	size_t nbytes, loff_t *ppos)
 {
-	struct dentry *dent = file->f_path.dentry;
 	struct lpfc_hba *phba = file->private_data;
+	int kind = debugfs_get_aux_num(file);
 	char cbuf[32];
 	uint64_t tmp = 0;
 	int cnt = 0;
 
-	if (dent == phba->debug_writeGuard)
+	if (kind == writeGuard)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wgrd_cnt);
-	else if (dent == phba->debug_writeApp)
+	else if (kind == writeApp)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wapp_cnt);
-	else if (dent == phba->debug_writeRef)
+	else if (kind == writeRef)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wref_cnt);
-	else if (dent == phba->debug_readGuard)
+	else if (kind == readGuard)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rgrd_cnt);
-	else if (dent == phba->debug_readApp)
+	else if (kind == readApp)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rapp_cnt);
-	else if (dent == phba->debug_readRef)
+	else if (kind == readRef)
 		cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rref_cnt);
-	else if (dent == phba->debug_InjErrNPortID)
+	else if (kind == InjErrNPortID)
 		cnt = scnprintf(cbuf, 32, "0x%06x\n",
 				phba->lpfc_injerr_nportid);
-	else if (dent == phba->debug_InjErrWWPN) {
+	else if (kind == InjErrWWPN) {
 		memcpy(&tmp, &phba->lpfc_injerr_wwpn, sizeof(struct lpfc_name));
 		tmp = cpu_to_be64(tmp);
 		cnt = scnprintf(cbuf, 32, "0x%016llx\n", tmp);
-	} else if (dent == phba->debug_InjErrLBA) {
+	} else if (kind == InjErrLBA) {
 		if (phba->lpfc_injerr_lba == (sector_t)(-1))
 			cnt = scnprintf(cbuf, 32, "off\n");
 		else
@@ -2417,8 +2417,8 @@ static ssize_t
 lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	size_t nbytes, loff_t *ppos)
 {
-	struct dentry *dent = file->f_path.dentry;
 	struct lpfc_hba *phba = file->private_data;
+	int kind = debugfs_get_aux_num(file);
 	char dstbuf[33];
 	uint64_t tmp = 0;
 	int size;
@@ -2428,7 +2428,7 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	if (copy_from_user(dstbuf, buf, size))
 		return -EFAULT;
 
-	if (dent == phba->debug_InjErrLBA) {
+	if (kind == InjErrLBA) {
 		if ((dstbuf[0] == 'o') && (dstbuf[1] == 'f') &&
 		    (dstbuf[2] == 'f'))
 			tmp = (uint64_t)(-1);
@@ -2437,23 +2437,23 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
 	if ((tmp == 0) && (kstrtoull(dstbuf, 0, &tmp)))
 		return -EINVAL;
 
-	if (dent == phba->debug_writeGuard)
+	if (kind == writeGuard)
 		phba->lpfc_injerr_wgrd_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_writeApp)
+	else if (kind == writeApp)
 		phba->lpfc_injerr_wapp_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_writeRef)
+	else if (kind == writeRef)
 		phba->lpfc_injerr_wref_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readGuard)
+	else if (kind == readGuard)
 		phba->lpfc_injerr_rgrd_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readApp)
+	else if (kind == readApp)
 		phba->lpfc_injerr_rapp_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_readRef)
+	else if (kind == readRef)
 		phba->lpfc_injerr_rref_cnt = (uint32_t)tmp;
-	else if (dent == phba->debug_InjErrLBA)
+	else if (kind == InjErrLBA)
 		phba->lpfc_injerr_lba = (sector_t)tmp;
-	else if (dent == phba->debug_InjErrNPortID)
+	else if (kind == InjErrNPortID)
 		phba->lpfc_injerr_nportid = (uint32_t)(tmp & Mask_DID);
-	else if (dent == phba->debug_InjErrWWPN) {
+	else if (kind == InjErrWWPN) {
 		tmp = cpu_to_be64(tmp);
 		memcpy(&phba->lpfc_injerr_wwpn, &tmp, sizeof(struct lpfc_name));
 	} else
@@ -6160,60 +6160,51 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
 			phba->debug_dumpHostSlim = NULL;
 
 		/* Setup DIF Error Injections */
-		snprintf(name, sizeof(name), "InjErrLBA");
 		phba->debug_InjErrLBA =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrLBA", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrLBA, &lpfc_debugfs_op_dif_err);
 		phba->lpfc_injerr_lba = LPFC_INJERR_LBA_OFF;
 
-		snprintf(name, sizeof(name), "InjErrNPortID");
 		phba->debug_InjErrNPortID =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrNPortID", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrNPortID, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "InjErrWWPN");
 		phba->debug_InjErrWWPN =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("InjErrWWPN", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, InjErrWWPN, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeGuardInjErr");
 		phba->debug_writeGuard =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeGuardInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeGuard, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeAppInjErr");
 		phba->debug_writeApp =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeAppInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeApp, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "writeRefInjErr");
 		phba->debug_writeRef =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("writeRefInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, writeRef, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readGuardInjErr");
 		phba->debug_readGuard =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readGuardInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readGuard, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readAppInjErr");
 		phba->debug_readApp =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readAppInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readApp, &lpfc_debugfs_op_dif_err);
 
-		snprintf(name, sizeof(name), "readRefInjErr");
 		phba->debug_readRef =
-			debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
+			debugfs_create_file_aux_num("readRefInjErr", 0644,
 			phba->hba_debugfs_root,
-			phba, &lpfc_debugfs_op_dif_err);
+			phba, readRef, &lpfc_debugfs_op_dif_err);
 
 		/* Setup slow ring trace */
 		if (lpfc_debugfs_max_slow_ring_trc) {
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.h b/drivers/scsi/lpfc/lpfc_debugfs.h
index 8d2e8d05bbc0..f319f3af0400 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.h
+++ b/drivers/scsi/lpfc/lpfc_debugfs.h
@@ -322,6 +322,17 @@ enum {
 						 * discovery */
 #endif /* H_LPFC_DEBUG_FS */
 
+enum {
+	writeGuard = 1,
+	writeApp,
+	writeRef,
+	readGuard,
+	readApp,
+	readRef,
+	InjErrLBA,
+	InjErrNPortID,
+	InjErrWWPN,
+};
 
 /*
  * Driver debug utility routines outside of debugfs. The debug utility

Re: [PATCH 11/11] lpfc: don't use file->f_path.dentry for comparisons

[Justin Tee]

> [PATCH v2] lpfc: don't use file->f_path.dentry for comparisons
>
> If you want a home-grown switch, at least use enum for selector...
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
> index 3fd1aa5cc78c..42d138ec11b4 100644
> --- a/drivers/scsi/lpfc/lpfc_debugfs.c
> +++ b/drivers/scsi/lpfc/lpfc_debugfs.c
> @@ -2375,32 +2375,32 @@ static ssize_t
>  lpfc_debugfs_dif_err_read(struct file *file, char __user *buf,
>         size_t nbytes, loff_t *ppos)
>  {
> -       struct dentry *dent = file->f_path.dentry;
>         struct lpfc_hba *phba = file->private_data;
> +       int kind = debugfs_get_aux_num(file);
>         char cbuf[32];
>         uint64_t tmp = 0;
>         int cnt = 0;
>
> -       if (dent == phba->debug_writeGuard)
> +       if (kind == writeGuard)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wgrd_cnt);
> -       else if (dent == phba->debug_writeApp)
> +       else if (kind == writeApp)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wapp_cnt);
> -       else if (dent == phba->debug_writeRef)
> +       else if (kind == writeRef)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_wref_cnt);
> -       else if (dent == phba->debug_readGuard)
> +       else if (kind == readGuard)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rgrd_cnt);
> -       else if (dent == phba->debug_readApp)
> +       else if (kind == readApp)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rapp_cnt);
> -       else if (dent == phba->debug_readRef)
> +       else if (kind == readRef)
>                 cnt = scnprintf(cbuf, 32, "%u\n", phba->lpfc_injerr_rref_cnt);
> -       else if (dent == phba->debug_InjErrNPortID)
> +       else if (kind == InjErrNPortID)
>                 cnt = scnprintf(cbuf, 32, "0x%06x\n",
>                                 phba->lpfc_injerr_nportid);
> -       else if (dent == phba->debug_InjErrWWPN) {
> +       else if (kind == InjErrWWPN) {
>                 memcpy(&tmp, &phba->lpfc_injerr_wwpn, sizeof(struct lpfc_name));
>                 tmp = cpu_to_be64(tmp);
>                 cnt = scnprintf(cbuf, 32, "0x%016llx\n", tmp);
> -       } else if (dent == phba->debug_InjErrLBA) {
> +       } else if (kind == InjErrLBA) {
>                 if (phba->lpfc_injerr_lba == (sector_t)(-1))
>                         cnt = scnprintf(cbuf, 32, "off\n");
>                 else
> @@ -2417,8 +2417,8 @@ static ssize_t
>  lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
>         size_t nbytes, loff_t *ppos)
>  {
> -       struct dentry *dent = file->f_path.dentry;
>         struct lpfc_hba *phba = file->private_data;
> +       int kind = debugfs_get_aux_num(file);
>         char dstbuf[33];
>         uint64_t tmp = 0;
>         int size;
> @@ -2428,7 +2428,7 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
>         if (copy_from_user(dstbuf, buf, size))
>                 return -EFAULT;
>
> -       if (dent == phba->debug_InjErrLBA) {
> +       if (kind == InjErrLBA) {
>                 if ((dstbuf[0] == 'o') && (dstbuf[1] == 'f') &&
>                     (dstbuf[2] == 'f'))
>                         tmp = (uint64_t)(-1);
> @@ -2437,23 +2437,23 @@ lpfc_debugfs_dif_err_write(struct file *file, const char __user *buf,
>         if ((tmp == 0) && (kstrtoull(dstbuf, 0, &tmp)))
>                 return -EINVAL;
>
> -       if (dent == phba->debug_writeGuard)
> +       if (kind == writeGuard)
>                 phba->lpfc_injerr_wgrd_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_writeApp)
> +       else if (kind == writeApp)
>                 phba->lpfc_injerr_wapp_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_writeRef)
> +       else if (kind == writeRef)
>                 phba->lpfc_injerr_wref_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_readGuard)
> +       else if (kind == readGuard)
>                 phba->lpfc_injerr_rgrd_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_readApp)
> +       else if (kind == readApp)
>                 phba->lpfc_injerr_rapp_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_readRef)
> +       else if (kind == readRef)
>                 phba->lpfc_injerr_rref_cnt = (uint32_t)tmp;
> -       else if (dent == phba->debug_InjErrLBA)
> +       else if (kind == InjErrLBA)
>                 phba->lpfc_injerr_lba = (sector_t)tmp;
> -       else if (dent == phba->debug_InjErrNPortID)
> +       else if (kind == InjErrNPortID)
>                 phba->lpfc_injerr_nportid = (uint32_t)(tmp & Mask_DID);
> -       else if (dent == phba->debug_InjErrWWPN) {
> +       else if (kind == InjErrWWPN) {
>                 tmp = cpu_to_be64(tmp);
>                 memcpy(&phba->lpfc_injerr_wwpn, &tmp, sizeof(struct lpfc_name));
>         } else
> @@ -6160,60 +6160,51 @@ lpfc_debugfs_initialize(struct lpfc_vport *vport)
>                         phba->debug_dumpHostSlim = NULL;
>
>                 /* Setup DIF Error Injections */
> -               snprintf(name, sizeof(name), "InjErrLBA");
>                 phba->debug_InjErrLBA =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("InjErrLBA", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, InjErrLBA, &lpfc_debugfs_op_dif_err);
>                 phba->lpfc_injerr_lba = LPFC_INJERR_LBA_OFF;
>
> -               snprintf(name, sizeof(name), "InjErrNPortID");
>                 phba->debug_InjErrNPortID =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("InjErrNPortID", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, InjErrNPortID, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "InjErrWWPN");
>                 phba->debug_InjErrWWPN =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("InjErrWWPN", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, InjErrWWPN, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "writeGuardInjErr");
>                 phba->debug_writeGuard =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("writeGuardInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, writeGuard, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "writeAppInjErr");
>                 phba->debug_writeApp =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("writeAppInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, writeApp, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "writeRefInjErr");
>                 phba->debug_writeRef =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("writeRefInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, writeRef, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "readGuardInjErr");
>                 phba->debug_readGuard =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("readGuardInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, readGuard, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "readAppInjErr");
>                 phba->debug_readApp =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("readAppInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, readApp, &lpfc_debugfs_op_dif_err);
>
> -               snprintf(name, sizeof(name), "readRefInjErr");
>                 phba->debug_readRef =
> -                       debugfs_create_file(name, S_IFREG|S_IRUGO|S_IWUSR,
> +                       debugfs_create_file_aux_num("readRefInjErr", 0644,
>                         phba->hba_debugfs_root,
> -                       phba, &lpfc_debugfs_op_dif_err);
> +                       phba, readRef, &lpfc_debugfs_op_dif_err);
>
>                 /* Setup slow ring trace */
>                 if (lpfc_debugfs_max_slow_ring_trc) {
> diff --git a/drivers/scsi/lpfc/lpfc_debugfs.h b/drivers/scsi/lpfc/lpfc_debugfs.h
> index 8d2e8d05bbc0..f319f3af0400 100644
> --- a/drivers/scsi/lpfc/lpfc_debugfs.h
> +++ b/drivers/scsi/lpfc/lpfc_debugfs.h
> @@ -322,6 +322,17 @@ enum {
>                                                  * discovery */
>  #endif /* H_LPFC_DEBUG_FS */
>
> +enum {
> +       writeGuard = 1,
> +       writeApp,
> +       writeRef,
> +       readGuard,
> +       readApp,
> +       readRef,
> +       InjErrLBA,
> +       InjErrNPortID,
> +       InjErrWWPN,
> +};
>
>  /*
>   * Driver debug utility routines outside of debugfs. The debug utility

Reviewed-by: Justin Tee <justin.tee@broadcom.com>

Regards,
Justin

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Jens Axboe]

On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
> When debugfs file has been created by debugfs_create_file_unsafe(),
> we do need the file_operations methods to use debugfs_file_{get,put}()
> to prevent concurrent removal; for files created by debugfs_create_file()
> that is done in the wrappers that call underlying methods, so there's
> no point whatsoever duplicating that in the underlying methods themselves.
> 
> 
> [...]

Applied, thanks!

[10/11] blk-mq-debugfs: use debugfs_get_aux()
        commit: c25885fc939f29200cccb58ffdb920a91ec62647

Best regards,
-- 
Jens Axboe

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Al Viro]

On Wed, Jul 02, 2025 at 05:19:12PM -0600, Jens Axboe wrote:
> 
> On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
> > When debugfs file has been created by debugfs_create_file_unsafe(),
> > we do need the file_operations methods to use debugfs_file_{get,put}()
> > to prevent concurrent removal; for files created by debugfs_create_file()
> > that is done in the wrappers that call underlying methods, so there's
> > no point whatsoever duplicating that in the underlying methods themselves.
> > 
> > 
> > [...]
> 
> Applied, thanks!
> 
> [10/11] blk-mq-debugfs: use debugfs_get_aux()
>         commit: c25885fc939f29200cccb58ffdb920a91ec62647

Umm...  That sucker depends upon the previous commit - you'll
need to cast debugfs_get_aux() result to void * without that...

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Jens Axboe]

On 7/2/25 6:23 PM, Al Viro wrote:
> On Wed, Jul 02, 2025 at 05:19:12PM -0600, Jens Axboe wrote:
>>
>> On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
>>> When debugfs file has been created by debugfs_create_file_unsafe(),
>>> we do need the file_operations methods to use debugfs_file_{get,put}()
>>> to prevent concurrent removal; for files created by debugfs_create_file()
>>> that is done in the wrappers that call underlying methods, so there's
>>> no point whatsoever duplicating that in the underlying methods themselves.
>>>
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [10/11] blk-mq-debugfs: use debugfs_get_aux()
>>         commit: c25885fc939f29200cccb58ffdb920a91ec62647
> 
> Umm...  That sucker depends upon the previous commit - you'll
> need to cast debugfs_get_aux() result to void * without that...

Gah ok - wasn't cleear since I wasn't CC'ed on the series, just the
single patch. If it's a single patch, I'm assuming it's good to go
if it looks good.

I'll just drop it.

-- 
Jens Axboe

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Greg Kroah-Hartman]

On Thu, Jul 03, 2025 at 01:23:29AM +0100, Al Viro wrote:
> On Wed, Jul 02, 2025 at 05:19:12PM -0600, Jens Axboe wrote:
> > 
> > On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
> > > When debugfs file has been created by debugfs_create_file_unsafe(),
> > > we do need the file_operations methods to use debugfs_file_{get,put}()
> > > to prevent concurrent removal; for files created by debugfs_create_file()
> > > that is done in the wrappers that call underlying methods, so there's
> > > no point whatsoever duplicating that in the underlying methods themselves.
> > > 
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [10/11] blk-mq-debugfs: use debugfs_get_aux()
> >         commit: c25885fc939f29200cccb58ffdb920a91ec62647
> 
> Umm...  That sucker depends upon the previous commit - you'll
> need to cast debugfs_get_aux() result to void * without that...

Wait, what "previous commit" this is patch 01/11 of the series?

I'm all for you just taking this through your trees if it depends on
something else that is there, but that wasn't obvious, sorry.

greg k-h

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Al Viro]

On Thu, Jul 03, 2025 at 01:37:58PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 03, 2025 at 01:23:29AM +0100, Al Viro wrote:
> > On Wed, Jul 02, 2025 at 05:19:12PM -0600, Jens Axboe wrote:
> > > 
> > > On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
> > > > When debugfs file has been created by debugfs_create_file_unsafe(),
> > > > we do need the file_operations methods to use debugfs_file_{get,put}()
> > > > to prevent concurrent removal; for files created by debugfs_create_file()
> > > > that is done in the wrappers that call underlying methods, so there's
> > > > no point whatsoever duplicating that in the underlying methods themselves.
> > > > 
> > > > 
> > > > [...]
> > > 
> > > Applied, thanks!
> > > 
> > > [10/11] blk-mq-debugfs: use debugfs_get_aux()
       ^^^^^
> > >         commit: c25885fc939f29200cccb58ffdb920a91ec62647
> > 
> > Umm...  That sucker depends upon the previous commit - you'll
> > need to cast debugfs_get_aux() result to void * without that...
> 
> Wait, what "previous commit" this is patch 01/11 of the series?

Jens replied to 01/11 saying that he'd grabbed 10/11, unfortunately
without 09/11...

Re: (subset) [PATCH 01/11] zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops

[Mark Brown]

On Wed, 02 Jul 2025 22:14:08 +0100, Al Viro wrote:
> When debugfs file has been created by debugfs_create_file_unsafe(),
> we do need the file_operations methods to use debugfs_file_{get,put}()
> to prevent concurrent removal; for files created by debugfs_create_file()
> that is done in the wrappers that call underlying methods, so there's
> no point whatsoever duplicating that in the underlying methods themselves.
> 
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[03/11] regmap: get rid of redundant debugfs_file_{get,put}()
        commit: 9f711c9321cffe3e03709176873c277fa911c366

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCHES] assorted debugfs stuff

[Greg Kroah-Hartman]

On Wed, Jul 02, 2025 at 10:13:05PM +0100, Al Viro wrote:
> 	A bit more of debugfs work; that stuff sits in
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.debugfs
> Individual patches in followups.  Please, review.
> 
> Several removals of pointless debugfs_file_{get,put}():
>       zynqmp: don't bother with debugfs_file_{get,put}() in proxied fops
>       hfi1: get rid of redundant debugfs_file_{get,put}()
>       regmap: get rid of redundant debugfs_file_{get,put}()
>       resctrl: get rid of pointless debugfs_file_{get,put}()
> Getting rid of the last remnants of debugfs_real_fops():
>       vmscan: don't bother with debugfs_real_fops()
>       netronome: don't bother with debugfs_real_fops()
>       debugfs: split short and full proxy wrappers, kill debugfs_real_fops()
> 
> Bogosities in drivers/thermal/testing:
>       fix tt_command_write():
> 1) unbalanced debugfs_file_get().  Not needed in the first place -
> file_operations are accessed only via debugfs_create_file(), so
> debugfs wrappers will take care of that itself.
> 2) kmalloc() for a buffer used only for duration of a function is not
> a problem, but for a buffer no longer than 16 bytes?
> 3) strstr() is for finding substrings; for finding a character there's
> strchr().
> 
> debugfs_get_aux() stuff:
>       debugfs_get_aux(): allow storing non-const void *
>       blk-mq-debugfs: use debugfs_aux_data()
>       lpfc: don't use file->f_path.dentry for comparisons
> If you want a home-grown switch, at least use enum for selector...

All now queued up for testing, thanks.

greg k-h