linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Mike Rapoport <mike.rapoport@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Pekka Enberg <penberg@kernel.org>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, rcu@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Josh Triplett <josh@joshtriplett.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Mike Rapoport <rppt@kernel.org>,
	Neeraj Upadhyay <quic_neeraju@quicinc.com>,
	Paolo Abeni <pabeni@redhat.com>,
	"Paul E. McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH 0/7] remove SLOB and allow kfree() with kmem_cache_alloc()
Date: Mon, 13 Mar 2023 12:31:47 -0400	[thread overview]
Message-ID: <20230313123147.6d28c47e@gandalf.local.home> (raw)
In-Reply-To: <ZA2gofYkXRcJ8cLA@kernel.org>

On Sun, 12 Mar 2023 11:51:29 +0200
Mike Rapoport <mike.rapoport@gmail.com> wrote:

> git grep -in slob still gives a couple of matches. I've dropped the
> irrelevant ones it it left me with these:
> 
> CREDITS:14:D: SLOB slab allocator
> kernel/trace/ring_buffer.c:358: * Also stolen from mm/slob.c. Thanks to Mathieu Desnoyers for pointing
> mm/Kconfig:251:    SLOB allocator and is not recommended for systems with more than
> mm/Makefile:25:KCOV_INSTRUMENT_slob.o := n
>  
> Except the comment in kernel/trace/ring_buffer.c all are trivial.
> 
> As for the comment in ring_buffer.c, it looks completely irrelevant at this
> point.
> 
> @Steve?

You want me to remember something I wrote almost 15 years ago? I think I
understand that comment as much as you do. Yeah, that was when I was still
learning to write comments for my older self to understand, and I failed
miserably!

But git history comes to the rescue. The commit that added that comment was:

ed56829cb3195 ("ring_buffer: reset buffer page when freeing")

This was at a time when it was suggested to me to use the struct page
directly in the ring buffer and where we could do fun "tricks" for
"performance". (I was never really for this, but I wasn't going to argue).

And the code in question then had:

/*
 * Also stolen from mm/slob.c. Thanks to Mathieu Desnoyers for pointing
 * this issue out.
 */
static inline void free_buffer_page(struct buffer_page *bpage)
{
        reset_page_mapcount(&bpage->page);
        bpage->page.mapping = NULL;
        __free_page(&bpage->page);
}


But looking at commit: e4c2ce82ca27 ("ring_buffer: allocate buffer page
pointer")

It was finally decided that method was not safe, and we should not be using
struct page but just allocate an actual page (much safer!).

I never got rid of the comment, which was more about that
"reset_page_mapcount()", and should have been deleted back then.

Just remove that comment. And you could even add:

Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Fixes: e4c2ce82ca27 ("ring_buffer: allocate buffer page pointer")

-- Steve

  reply	other threads:[~2023-03-13 16:33 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 10:32 [PATCH 0/7] remove SLOB and allow kfree() with kmem_cache_alloc() Vlastimil Babka
2023-03-10 10:32 ` [PATCH 1/7] mm/slob: remove CONFIG_SLOB Vlastimil Babka
2023-03-14  7:18   ` Hyeonggon Yoo
2023-03-14 22:11   ` Lorenzo Stoakes
2023-03-10 10:32 ` [PATCH 2/7] net: skbuff: remove SLOB-specific ifdefs Vlastimil Babka
2023-03-10 10:32 ` [PATCH 3/7] mm, page_flags: remove PG_slob_free Vlastimil Babka
2023-03-14  7:25   ` Hyeonggon Yoo
2023-03-14 22:12   ` Lorenzo Stoakes
2023-03-10 10:32 ` [PATCH 4/7] mm, pagemap: remove SLOB and SLQB from comments and documentation Vlastimil Babka
2023-03-14  8:19   ` Hyeonggon Yoo
2023-03-15 11:05     ` Vlastimil Babka
2023-03-14 22:16   ` Lorenzo Stoakes
2023-03-10 10:32 ` [PATCH 5/7] mm/slab: remove CONFIG_SLOB code from slab common code Vlastimil Babka
2023-03-14  9:28   ` Hyeonggon Yoo
2023-03-14 22:16     ` Lorenzo Stoakes
2023-03-10 10:32 ` [PATCH 6/7] mm/slob: remove slob.c Vlastimil Babka
2023-03-14  9:34   ` Hyeonggon Yoo
2023-03-14 22:18   ` Lorenzo Stoakes
2023-03-15  2:54   ` Roman Gushchin
2023-03-10 10:32 ` [PATCH 7/7] mm/slab: document kfree() as allowed for kmem_cache_alloc() objects Vlastimil Babka
2023-03-12  9:59   ` Mike Rapoport
2023-03-15 13:38     ` Vlastimil Babka
2023-03-15 14:50       ` Mike Rapoport
2023-03-11  1:00 ` [PATCH 0/7] remove SLOB and allow kfree() with kmem_cache_alloc() Jakub Kicinski
2023-03-12  9:51 ` Mike Rapoport
2023-03-13 16:31   ` Steven Rostedt [this message]
2023-03-13 18:00     ` Mike Rapoport
2023-03-15 13:53     ` Vlastimil Babka
2023-03-15 14:20       ` Steven Rostedt
2023-03-15 14:22         ` Vlastimil Babka
2023-03-13 16:36   ` Vlastimil Babka
2023-03-14 22:10     ` Lorenzo Stoakes
2023-03-15 13:40       ` 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=20230313123147.6d28c47e@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=frederic@kernel.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kuba@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mike.rapoport@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=patches@lists.linux.dev \
    --cc=paulmck@kernel.org \
    --cc=penberg@kernel.org \
    --cc=quic_neeraju@quicinc.com \
    --cc=rcu@vger.kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rppt@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).