public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>,
	akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, david.hunter.linux@gmail.com,
	linux-kernel-mentees@lists.linuxfoundation.org,
	khalid@kernel.org, Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] lib: cpu_rmap.c Refactor allocation size calculation in kzalloc()
Date: Fri, 10 Oct 2025 11:00:53 -0600	[thread overview]
Message-ID: <bd60a8e5-f4df-4743-8b56-3d6127906ff2@linuxfoundation.org> (raw)
In-Reply-To: <84bf5902-b3e5-4d58-a2a7-f01e15cfe143@gmail.com>

On 10/9/25 09:16, Mehdi Ben Hadj Khelifa wrote:
> On 10/7/25 11:23 PM, Shuah Khan wrote:
> 
>>
>> How did you find this problem and how did you test this change?

Bummer - you trimmed the code entirely from the thread. Next time
leave it in for context for the discussion.

> For the first part of your question,After simply referring to deprecated documentation[1] which states the following:

Looks you forgot to add link to the deprecated documentation[1].
It sounds like this is a potential problem without a reproducer.
These types of problems made to a critical piece of code require
substantial testing.

> 'For other calculations, please compose the use of the size_mul(), size_add(), and size_sub() helpers'
> Which is about dynamic calculations made inside of kzalloc() and kmalloc(). Specifically, the quoted part is talking about calculations which can't be simply divided into two parameters referring to the number of elements and size per element and in cases where we can't use struct_size() too.After that it was a matter of finding code where that could be the problem which is the case of the changed code.
> 
> For the second part, As per any patch,I make a copy of all dmesg warnings errors critical messages,then I compile install and boot the new kernel then check if there is any change or regression in dmesg.

This is a basic boot test which isn't sufficient in this case.

> For this particular change, since it doesn't have any selftests because it's in utility library which in my case cpu_rmap is used in the networking subsystem, I did some fault injection with a custom module to test if in case of overflow it fails safely reporting the issue in dmesg which is catched by the __alloc_frozen_pages_noprof() function in mm/page_alloc.c and also return a NULL for rmap instead of wrapping to a smaller size.

Custom module testing doesn't test this change in a wider scope
which is necessary when you are making changes such as these
without a reproducer and a way to reproduce. How do you know
this change doesn't introduce regressions?

thanks,
-- Shuah

  reply	other threads:[~2025-10-10 17:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-30  9:23 [PATCH] lib: cpu_rmap.c Refactor allocation size calculation in kzalloc() Mehdi Ben Hadj Khelifa
2025-10-07 22:23 ` Shuah Khan
2025-10-09 15:16   ` Mehdi Ben Hadj Khelifa
2025-10-10 17:00     ` Shuah Khan [this message]
2025-10-18 16:52       ` Mehdi Ben Hadj Khelifa
2025-10-20 20:36         ` Shuah Khan

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=bd60a8e5-f4df-4743-8b56-3d6127906ff2@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=khalid@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mehdi.benhadjkhelifa@gmail.com \
    /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