On Sun, 12 Mar 2023 11:51:29 +0200
Mike Rapoport <mike.rapoport@xxxxxxxxx> 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@xxxxxxxxxxx>
Fixes: e4c2ce82ca27 ("ring_buffer: allocate buffer page pointer")
-- Steve
