Re: [PATCH v2 01/22] x86/mm: split out preallocate_sub_pgd()

[Brendan Jackman]

On Tue Mar 24, 2026 at 3:27 PM UTC, Borislav Petkov wrote:
> On Fri, Mar 20, 2026 at 06:23:25PM +0000, Brendan Jackman wrote:
>> This code will be needed elsewhere in a following patch. Split out the
>> trivial code move for easy review.
>> 
>> This changes the logging slightly: instead of panic() directly reporting
>> the level of the failure, there is now a generic panic message which
>> will be preceded by a separate warn that reports the level of the
>> failure. This is a simple way to have this helper suit the needs of its
>> new user as well as the existing one.
>
> "Describe your changes in imperative mood, e.g. “make xyzzy do frotz” instead
> of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to do frotz”, as
> if you are giving orders to the codebase to change its behaviour."

Yeah there are a bunch of comments in this series where I've written "we
do XYZ" instead of "do XYZ", I've set up an AI prompt [0] to try and
catch it for me before I send you guys crazy with yet another series
that makes the same mistake over and over.

[0] https://lore.kernel.org/all/DHA6G1E2H5P4.2D7JKTRKIBE3U@xxxxxxxxxx/

TBH for this particular case it seems borderline? The referent of the
"this" is the previous paragraph, it's trying to show that the logging
change is a side effect of the code movement rather than a part of
the patch's intent. But there are more explicit ways to carry that
message so I'll rewrite it.

>> Other than logging, no functional change intended.
>> 
>> Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
>> ---
>>  arch/x86/include/asm/pgalloc.h | 33 +++++++++++++++++++++++++++++++
>>  arch/x86/mm/init_64.c          | 44 +++++++-----------------------------------
>>  2 files changed, 40 insertions(+), 37 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
>> index c88691b15f3c6..3541b86c9c6b0 100644
>> --- a/arch/x86/include/asm/pgalloc.h
>> +++ b/arch/x86/include/asm/pgalloc.h
>
> Why in a header?
>
> That function is kinda bigger than the rest of the oneliners there...

Hm, I suspect I wanted to avoid creating new ifdefs in the C file? Seems
harmless though. Looks fine in arch/x86/mm/pgtable.c.