Re: [RFC][PATCH] fix io_uring_show_fdinfo() misuse of ->d_iname

[Jens Axboe <axboe@kernel.dk>]

On 1/17/25 7:57 PM, Al Viro wrote:
> 	The output of io_uring_show_fdinfo() is crazy - for
> each slot of io_uring file_table it produces either
> INDEX: <none>
> or
> INDEX: NAME
> where INDEX runs through all numbers from 0 to ctx->file_table.data.nr-1
> and NAME is usually the last component of pathname of file in slot
> #INDEX.  Usually == if it's no longer than 39 bytes.  If it's longer,
> you get junk.  Oh, and if it contains newlines, you get several lines and
> no way to tell that it has happened, them's the breaks.  If it's happens
> to be /home/luser/<none>, well, what you see is indistinguishable from what
> you'd get if it hadn't been there...
> 
> According to Jens, it's *not* cast in stone, so we should be able to
> change that to something saner.  I see two options:
> 
> 1) replace NAME with actual pathname of the damn file, quoted to reasonable
> extent.
> 
> 2) same, and skip the INDEX: <none> lines.  It's not as if they contained
> any useful information - the size of table is printed right before that,
> so you'd get
> 
> ...
> UserFiles:	16
>     0: foo
>    11: bar
> UserBufs:	....
> 
> instead of
> 
> ...
> UserFiles:	16
>     0: foo
>     1: <none>
>     2: <none>
>     3: <none>
>     4: <none>
>     5: <none>
>     6: <none>
>     7: <none>
>     8: <none>
>     9: <none>
>    10: <none>
>    11:	bar
>    12: <none>
>    13: <none>
>    14: <none>
>    15: <none>
> UserBufs:	....
> 
> IMO the former is more useful for any debugging purposes.
> 
> The patch is trivial either way - (1) is
> ------------------------
> diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> index b214e5a407b5..1017249ae610 100644
> --- a/io_uring/fdinfo.c
> +++ b/io_uring/fdinfo.c
> @@ -211,10 +211,12 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
>  
>  		if (ctx->file_table.data.nodes[i])
>  			f = io_slot_file(ctx->file_table.data.nodes[i]);
> +		seq_printf(m, "%5u: ", i);
>  		if (f)
> -			seq_printf(m, "%5u: %s\n", i, file_dentry(f)->d_iname);
> +			seq_file_path(m, f, " \t\n\\<");
>  		else
> -			seq_printf(m, "%5u: <none>\n", i);
> +			seq_puts(m, "<none>");
> +		seq_puts(m, "\n");
>  	}
>  	seq_printf(m, "UserBufs:\t%u\n", ctx->buf_table.nr);
>  	for (i = 0; has_lock && i < ctx->buf_table.nr; i++) {
> ------------------------
> and (2) -
> ------------------------
> diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> index b214e5a407b5..f60d0a9d505e 100644
> --- a/io_uring/fdinfo.c
> +++ b/io_uring/fdinfo.c
> @@ -211,10 +211,11 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
>  
>  		if (ctx->file_table.data.nodes[i])
>  			f = io_slot_file(ctx->file_table.data.nodes[i]);
> -		if (f)
> -			seq_printf(m, "%5u: %s\n", i, file_dentry(f)->d_iname);
> -		else
> -			seq_printf(m, "%5u: <none>\n", i);
> +		if (f) {
> +			seq_printf(m, "%5u: ", i);
> +			seq_file_path(m, f, " \t\n\\");
> +			seq_puts(m, "\n");
> +		}
>  	}
>  	seq_printf(m, "UserBufs:\t%u\n", ctx->buf_table.nr);
>  	for (i = 0; has_lock && i < ctx->buf_table.nr; i++) {
> ------------------------
> 
> Preferences?  The difference in seq_printf() argument is due to the need
> to quote < in filenames if we need to distinguish them from <none>;
> whitespace and \ needs to be quoted in either case.

I like #2, there's no reason to dump the empty nodes. Thanks Al!

-- 
Jens Axboe