diff mbox series

[RESEND,RFC] blkcg: rewind seq_file if no stats

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

Commit Message

Khazhy Kumykov May 13, 2022, 5:40 p.m. UTC
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(+)

Comments

Khazhy Kumykov May 13, 2022, 6:06 p.m. UTC | #1
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;
Christoph Hellwig May 17, 2022, 6:12 a.m. UTC | #2
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.
Khazhy Kumykov May 17, 2022, 6:27 a.m. UTC | #3
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 mbox series

Patch

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,