Message ID | 20220513174030.1307720-1-khazhy@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RESEND,RFC] blkcg: rewind seq_file if no stats | expand |
On Fri, May 13, 2022 at 10:40 AM Khazhismel Kumykov <khazhy@google.com> wrote: > +void seq_restore(struct seq_file *m, int count) (Yes, this and elsewhere should be size_t to match m->count) > +{ > + if (WARN_ON_ONCE(count > m->count || count > m->size)) > + return; > + m->count = count;
On Fri, May 13, 2022 at 10:40:30AM -0700, Khazhismel Kumykov wrote: > Restores the previous behavior of only displaying devices for which we > have statistics (and removes the current, broken, behavior of printing > devname with no newline if no statistics) > > In lieu of get_seq_buf + seq_commit, provide a way to "undo" writes if > we use seq_printf I have to say I much prefer the simpler fix from Wolfgang. But if we want to go down this route it needs to split into a patch for the seq_file infrastruture and a separate one for the blk-cgroup code.
On Mon, May 16, 2022 at 11:12 PM Christoph Hellwig <hch@lst.de> wrote: > > On Fri, May 13, 2022 at 10:40:30AM -0700, Khazhismel Kumykov wrote: > > Restores the previous behavior of only displaying devices for which we > > have statistics (and removes the current, broken, behavior of printing > > devname with no newline if no statistics) > > > > In lieu of get_seq_buf + seq_commit, provide a way to "undo" writes if > > we use seq_printf > > I have to say I much prefer the simpler fix from Wolfgang. But Wolfgang's fix still results in a change in behavior, and in my opinion a worse behavior - we don't need to print out the device names if there are no stats. > if we want to go down this route it needs to split into a patch > for the seq_file infrastruture and a separate one for the blk-cgroup > code. Of course - I can spin this into a series if we think it's worthwhile
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 8dfe62786cd5..50043a742c48 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -909,6 +909,7 @@ static void blkcg_print_one_stat(struct blkcg_gq *blkg, struct seq_file *s) const char *dname; unsigned seq; int i; + int scookie; if (!blkg->online) return; @@ -917,6 +918,8 @@ static void blkcg_print_one_stat(struct blkcg_gq *blkg, struct seq_file *s) if (!dname) return; + scookie = seq_checkpoint(s); + seq_printf(s, "%s ", dname); do { @@ -956,6 +959,8 @@ static void blkcg_print_one_stat(struct blkcg_gq *blkg, struct seq_file *s) if (has_stats) seq_printf(s, "\n"); + else + seq_restore(s, scookie); } static int blkcg_print_stat(struct seq_file *sf, void *v) diff --git a/fs/seq_file.c b/fs/seq_file.c index 7ab8a58c29b6..c3ec6b57334e 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -408,6 +408,20 @@ void seq_printf(struct seq_file *m, const char *f, ...) } EXPORT_SYMBOL(seq_printf); +int seq_checkpoint(struct seq_file *m) +{ + return m->count; +} +EXPORT_SYMBOL(seq_checkpoint); + +void seq_restore(struct seq_file *m, int count) +{ + if (WARN_ON_ONCE(count > m->count || count > m->size)) + return; + m->count = count; +} +EXPORT_SYMBOL(seq_restore); + #ifdef CONFIG_BINARY_PRINTF void seq_bprintf(struct seq_file *m, const char *f, const u32 *binary) { diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h index 60820ab511d2..d3a05f7c2750 100644 --- a/include/linux/seq_file.h +++ b/include/linux/seq_file.h @@ -117,6 +117,8 @@ __printf(2, 0) void seq_vprintf(struct seq_file *m, const char *fmt, va_list args); __printf(2, 3) void seq_printf(struct seq_file *m, const char *fmt, ...); +int seq_checkpoint(struct seq_file *m); +void seq_restore(struct seq_file *m, int count); void seq_putc(struct seq_file *m, char c); void seq_puts(struct seq_file *m, const char *s); void seq_put_decimal_ull_width(struct seq_file *m, const char *delimiter,
Restores the previous behavior of only displaying devices for which we have statistics (and removes the current, broken, behavior of printing devname with no newline if no statistics) In lieu of get_seq_buf + seq_commit, provide a way to "undo" writes if we use seq_printf Fixes: 252c651a4c85 ("blk-cgroup: stop using seq_get_buf") Signed-off-by: Khazhismel Kumykov <khazhy@google.com> --- block/blk-cgroup.c | 5 +++++ fs/seq_file.c | 14 ++++++++++++++ include/linux/seq_file.h | 2 ++ 3 files changed, 21 insertions(+)