All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
To: Faiyaz Mohammed <faiyazm@codeaurora.org>
Cc: linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: [RFC PATCH 0/1] mm/slub: endless "No data" printing for alloc/free_traces attribute
Date: Wed, 17 Nov 2021 20:39:31 +0100	[thread overview]
Message-ID: <20211117193932.4049412-1-gerald.schaefer@linux.ibm.com> (raw)

Steffen reported endless printing of "No data" when reading from
alloc/free_traces attribute in /sys/kernel/debug/slab/, at least on
s390, but I don't see how this could be arch-specific.

I must admit that I am completely confused by the seq_file interface
in general, but even more so from this specific implementation.

First I suspected a bug in slab_debugfs_next(), because of its very
weird usage of the *v parameter, which seems totally useless, but not
responsible for this bug. Still, out of curiosity, does anybody have a
clue why it is done this way, and not e.g. like this?

diff --git a/mm/slub.c b/mm/slub.c
index f7368bfffb7a..0b1832b16f5b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6105,10 +6105,9 @@ static void *slab_debugfs_next(struct seq_file *seq, void *v, loff_t *ppos)
 {
        struct loc_track *t = seq->private;

-       v = ppos;
        ++*ppos;
        if (*ppos <= t->count)
-               return v;
+               return ppos;

        return NULL;
 }

Anyway, NULL is returned for the "*ppos > t->count" case, as described
in Documentation/filesystems/seq_file.rst, for "if the sequence is
complete", so that should be ok, but still very confusing. Unfortunately,
the documentation does not seem to explain that *v parameter at all, or
I missed it, but in this case here it seems to be simply ignored and
misused w/o any need.

The documentation does mention that "in most cases the start() function
should check for a "past end of file" condition and return NULL if need
be". So I just added a similar check to slab_debugfs_start() and return
NULL for "*ppos > t->count", which apparently fixed the bug. "No data"
is now only printed once, like it was before commit 64dd68497be7
("mm: slub: move sysfs slab alloc/free interfaces to debugfs").

However, since I obviously do not really understand the seq_file
documentation, and also not the alloc/free_traces stuff in general,
that fix might be wrong or incomplete. Comments are welcome.

Gerald Schaefer (1):
  mm/slub: fix endless "No data" printing for alloc/free_traces
    attribute

 mm/slub.c | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.25.1


             reply	other threads:[~2021-11-17 19:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17 19:39 Gerald Schaefer [this message]
2021-11-17 19:39 ` [RFC PATCH 1/1] mm/slub: fix endless "No data" printing for alloc/free_traces attribute Gerald Schaefer
2021-11-19 10:41   ` Vlastimil Babka
2021-11-19 19:59     ` Gerald Schaefer
2021-11-22 15:02       ` Vlastimil Babka
2021-11-22 20:14         ` Gerald Schaefer
2021-11-22 20:33           ` Gerald Schaefer
2021-11-23 14:19             ` Vlastimil Babka
2021-11-25 16:13               ` Gerald Schaefer
2021-11-25 20:12                 ` Gerald Schaefer
2021-11-25 22:00                   ` Vlastimil Babka
2021-11-26 17:11                     ` Gerald Schaefer
2021-11-26 17:18                     ` [PATCH] mm/slub: fix endianness bug for alloc/free_traces attributes Gerald Schaefer
2021-11-29 14:26                       ` Vlastimil Babka
2021-11-19  9:43 ` [RFC PATCH 0/1] mm/slub: endless "No data" printing for alloc/free_traces attribute Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211117193932.4049412-1-gerald.schaefer@linux.ibm.com \
    --to=gerald.schaefer@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=faiyazm@codeaurora.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.