linux-kernel - Re: [PATCH 1/4] mm/memory_hotplug: Update comment for hotplug memory callback priorities

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: <3e48429a-b52d-43a1-b48a-06fb46f0a37c@linux.intel.com>
Date: Mon, 18 Aug 2025 20:14:45 -0700
From: Marc Herbert <marc.herbert@...ux.intel.com>
To: Dave Jiang <dave.jiang@...el.com>, David Hildenbrand <david@...hat.com>,
 linux-cxl@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, dakr@...nel.org,
 dave@...olabs.net, jonathan.cameron@...wei.com, alison.schofield@...el.com,
 vishal.l.verma@...el.com, ira.weiny@...el.com, dan.j.williams@...el.com,
 akpm@...ux-foundation.org
Subject: Re: [PATCH 1/4] mm/memory_hotplug: Update comment for hotplug memory
 callback priorities



On 2025-08-18 07:08, Dave Jiang wrote:
> 
> 
> On 8/16/25 12:29 AM, David Hildenbrand wrote:
>> On 14.08.25 19:16, Dave Jiang wrote:
>>> Add clarification to comment for memory hotplug callback ordering as the
>>> current comment does not provide clear language on which callback happens
>>> first.
>>>
>>> Signed-off-by: Dave Jiang <dave.jiang@...el.com>
>>> ---
>>>   include/linux/memory.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/memory.h b/include/linux/memory.h
>>> index 40eb70ccb09d..02314723e5bd 100644
>>> --- a/include/linux/memory.h
>>> +++ b/include/linux/memory.h
>>> @@ -116,7 +116,7 @@ struct mem_section;
>>>     /*
>>>    * Priorities for the hotplug memory callback routines (stored in decreasing
>>> - * order in the callback chain)
>>> + * order in the callback chain). The callback ordering happens from high to low.
>>>    */
>>>   #define DEFAULT_CALLBACK_PRI    0
>>>   #define SLAB_CALLBACK_PRI    1
>>
>> "stored in decreasing order in the callback chain"
>>
>> is pretty clear? It's a chain after all that gets called.
> 
> I can drop the patch. For some reason when I read it I'm thinking the opposite, and when Marc was also confused I started questioning things.
> 

I think we both found the current comment confusing (even together!)
because:

- It very briefly alludes to an implementation detail (the chain)
  without really getting into detail. A "chain" could be bi-directional;
  why not? This one is... "most likely" not. Doubt.

- Higher priorities can have lower numbers, example: "P1 bugs". Not the
  case here, but this "double standards" situation makes _all_
  priorities suspicious and confusing.

- Constants that come first in the file are called last.


I would go further than Dave and also drop the "chain" implementation
detail because it makes the reader to think too much.  Not needed and
distracting at this particular point in the file.

/*
 * Priorities for the hotplug memory callback routines.
 * Invoked from high to low.
 */

  => Hopefully zero cognitive load.