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
