linux-kernel - Re: [PATCH v4] mm/page_alloc: Consolidate unlikely handling in page_expected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-q71LlcCQ5I-2D-@casper.infradead.org>
Date: Mon, 31 Mar 2025 16:59:16 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Ye Liu <ye.liu@...ux.dev>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Markus.Elfring@....de,
	Ye Liu <liuye@...inos.cn>,
	Sidhartha Kumar <sidhartha.kumar@...cle.com>,
	Anshuman Khandual <anshuman.khandual@....com>
Subject: Re: [PATCH v4] mm/page_alloc: Consolidate unlikely handling in
 page_expected_state

On Mon, Mar 31, 2025 at 08:08:01PM +0800, Ye Liu wrote:
> 
> 在 2025/3/28 22:29, Matthew Wilcox 写道:
> > On Fri, Mar 28, 2025 at 09:47:57AM +0800, Ye Liu wrote:
> >> Consolidate the handling of unlikely conditions in the 
> >> page_expected_state() function to reduce code duplication and improve 
> >> readability.
> > I don't think this is an equivalent transformation.
> Could you explain it in detail?

page_expected_state() is called both at free and alloc.  I think
the correct behaviour on encountering a HWPOISON page should be
different at alloc and free, don't you?

> > Please, stop with these tweaky patches to incredibly sensitive core code.
> > Fix a problem, or leave it alone.  We are primarily short of reviewer
> > bandwidth.  You could help with that by reviewing other people's patches.
> > Sending patches of your own just adds to other people's workload.
> Thank you for your feedback. I understand the sensitivity of core code
> and respect the limitations on reviewer bandwidth. However, I believe
> that reasonable optimizations should not be rejected solely because
> they involve core code. If an improvement enhances performance,
> readability, or maintainability without introducing risks, wouldn't
> it be worth considering for review?

If it's a reasonable optimisation, absolutely!  But if it's an
optimisation, it should be accompanied with a benchmark showing an
improvement.  As far as improving readability, I'm not yet convinced
that you have the expertise to make that call.  Every change that is
made invalidates everybody else's mental model of "how this works".
So all changes carry a cost.  Sometimes that cost is worth paying,
other times it isn't.

> Regarding the reviewer shortage, I’d be happy to help by reviewing
> other patches as well. Could you please share the process for becoming
> a reviewer? What are the requirements or steps to get involved?

There is no process!  Choose a patch, read it, think about it.  What
problems might there be with it?  What may have been overlooked?
Is the commit message unclear to you, how could it be improved?
When you're done, send a Reviewed-by: tag (read the kernel process
documents for the full meaning of that tag).