| Message ID | 20240812020052.8763-1-ruyi.zhang@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring/fdinfo: add timeout_list to fdinfo | expand |
On 8/11/24 8:00 PM, Ruyi Zhang wrote: > io_uring fdinfo contains most of the runtime information, > which is helpful for debugging io_uring applications; > However, there is currently a lack of timeout-related > information, and this patch adds timeout_list information. Please wrap this at 72 chars, lines are too short right now. > @@ -219,9 +221,19 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file) > > seq_printf(m, " user_data=%llu, res=%d, flags=%x\n", > cqe->user_data, cqe->res, cqe->flags); > - > } > - > spin_unlock(&ctx->completion_lock); Some unrelated whitespace changes here? > + > + seq_puts(m, "TimeoutList:\n"); > + spin_lock(&ctx->timeout_lock); > + list_for_each_entry(timeout, &ctx->timeout_list, list) { > + struct io_kiocb *req = cmd_to_io_kiocb(timeout); > + struct io_timeout_data *data = req->async_data; > + > + seq_printf(m, " off=%d, target_seq=%d, repeats=%x, ts.tv_sec=%lld, ts.tv_nsec=%ld\n", > + timeout->off, timeout->target_seq, timeout->repeats, > + data->ts.tv_sec, data->ts.tv_nsec); > + } > + spin_unlock(&ctx->timeout_lock); That seq_printf() line is way too long, please wrap like what is done for the overflow printing above. Outside of that, please also rebase on the for-6.12/io_uring branch, as this one would not apply there. Thanks!
On 8/12/24 03:00, Ruyi Zhang wrote: > io_uring fdinfo contains most of the runtime information, > which is helpful for debugging io_uring applications; > However, there is currently a lack of timeout-related > information, and this patch adds timeout_list information. > > Signed-off-by: Ruyi Zhang <ruyi.zhang@samsung.com> > --- > io_uring/fdinfo.c | 16 ++++++++++++++-- > io_uring/timeout.c | 12 ------------ > io_uring/timeout.h | 12 ++++++++++++ > 3 files changed, 26 insertions(+), 14 deletions(-) > > diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c > index b1e0e0d85349..33c3efd79f98 100644 > --- a/io_uring/fdinfo.c > +++ b/io_uring/fdinfo.c > @@ -14,6 +14,7 @@ > #include "fdinfo.h" > #include "cancel.h" > #include "rsrc.h" > +#include "timeout.h" > > #ifdef CONFIG_PROC_FS > static __cold int io_uring_show_cred(struct seq_file *m, unsigned int id, > @@ -54,6 +55,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file) > { > struct io_ring_ctx *ctx = file->private_data; > struct io_overflow_cqe *ocqe; > + struct io_timeout *timeout; > struct io_rings *r = ctx->rings; > struct rusage sq_usage; > unsigned int sq_mask = ctx->sq_entries - 1, cq_mask = ctx->cq_entries - 1; > @@ -219,9 +221,19 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file) > > seq_printf(m, " user_data=%llu, res=%d, flags=%x\n", > cqe->user_data, cqe->res, cqe->flags); > - > } > - > spin_unlock(&ctx->completion_lock); > + > + seq_puts(m, "TimeoutList:\n"); > + spin_lock(&ctx->timeout_lock); _irq > + list_for_each_entry(timeout, &ctx->timeout_list, list) { > + struct io_kiocb *req = cmd_to_io_kiocb(timeout); > + struct io_timeout_data *data = req->async_data; > + I'd argue we don't want it, there should be better way for reflection. And we also don't want to walk a potentially very long list under spinlock without IRQs, especially from procfs path, and even more so with seq_printf in there doing a lot of work. Yes, we already walk the list like that for cancellation, but it's lighter than seq_printf, and we should be moving in the direction of improving it, not aggravating the situation. > + seq_printf(m, " off=%d, target_seq=%d, repeats=%x, ts.tv_sec=%lld, ts.tv_nsec=%ld\n", > + timeout->off, timeout->target_seq, timeout->repeats, > + data->ts.tv_sec, data->ts.tv_nsec); We should be deprecating sequences, i.e. target_seq, not exposing it further to the user. > + } > + spin_unlock(&ctx->timeout_lock); > } > #endif
diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c index b1e0e0d85349..33c3efd79f98 100644 --- a/io_uring/fdinfo.c +++ b/io_uring/fdinfo.c @@ -14,6 +14,7 @@ #include "fdinfo.h" #include "cancel.h" #include "rsrc.h" +#include "timeout.h" #ifdef CONFIG_PROC_FS static __cold int io_uring_show_cred(struct seq_file *m, unsigned int id, @@ -54,6 +55,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file) { struct io_ring_ctx *ctx = file->private_data; struct io_overflow_cqe *ocqe; + struct io_timeout *timeout; struct io_rings *r = ctx->rings; struct rusage sq_usage; unsigned int sq_mask = ctx->sq_entries - 1, cq_mask = ctx->cq_entries - 1; @@ -219,9 +221,19 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file) seq_printf(m, " user_data=%llu, res=%d, flags=%x\n", cqe->user_data, cqe->res, cqe->flags); - } - spin_unlock(&ctx->completion_lock); + + seq_puts(m, "TimeoutList:\n"); + spin_lock(&ctx->timeout_lock); + list_for_each_entry(timeout, &ctx->timeout_list, list) { + struct io_kiocb *req = cmd_to_io_kiocb(timeout); + struct io_timeout_data *data = req->async_data; + + seq_printf(m, " off=%d, target_seq=%d, repeats=%x, ts.tv_sec=%lld, ts.tv_nsec=%ld\n", + timeout->off, timeout->target_seq, timeout->repeats, + data->ts.tv_sec, data->ts.tv_nsec); + } + spin_unlock(&ctx->timeout_lock); } #endif diff --git a/io_uring/timeout.c b/io_uring/timeout.c index 9973876d91b0..4449e139e371 100644 --- a/io_uring/timeout.c +++ b/io_uring/timeout.c @@ -13,18 +13,6 @@ #include "cancel.h" #include "timeout.h" -struct io_timeout { - struct file *file; - u32 off; - u32 target_seq; - u32 repeats; - struct list_head list; - /* head of the link, used by linked timeouts only */ - struct io_kiocb *head; - /* for linked completions */ - struct io_kiocb *prev; -}; - struct io_timeout_rem { struct file *file; u64 addr; diff --git a/io_uring/timeout.h b/io_uring/timeout.h index a6939f18313e..befd489a6286 100644 --- a/io_uring/timeout.h +++ b/io_uring/timeout.h @@ -1,5 +1,17 @@ // SPDX-License-Identifier: GPL-2.0 +struct io_timeout { + struct file *file; + u32 off; + u32 target_seq; + u32 repeats; + struct list_head list; + /* head of the link, used by linked timeouts only */ + struct io_kiocb *head; + /* for linked completions */ + struct io_kiocb *prev; +}; + struct io_timeout_data { struct io_kiocb *req; struct hrtimer timer;
io_uring fdinfo contains most of the runtime information, which is helpful for debugging io_uring applications; However, there is currently a lack of timeout-related information, and this patch adds timeout_list information. Signed-off-by: Ruyi Zhang <ruyi.zhang@samsung.com> --- io_uring/fdinfo.c | 16 ++++++++++++++-- io_uring/timeout.c | 12 ------------ io_uring/timeout.h | 12 ++++++++++++ 3 files changed, 26 insertions(+), 14 deletions(-)