On 19.08.25 17:39, Dave Jiang wrote:
On 8/19/25 2:18 AM, David Hildenbrand wrote:
On 19.08.25 05:14, Marc Herbert wrote:
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@xxxxxxxxx>
---
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.
Please note that the memory notifier is really just using the generic *notifier chain* mechanism as documented in include/linux/notifier.h.
Here is a good summary of that mechanism. I don't quite agree with the "implementation detail" part, but that information might indeed not be required here.
https://0xax.gitbooks.io/linux-insides/content/Concepts/linux-cpu-4.html
- Higher priorities can have lower numbers, example: "P1 bugs". Not the
case here, but this "double standards" situation makes _all_
priorities suspicious and confusing.
Yes, "priorities" are handled differently in different context.
- Constants that come first in the file are called last.
Yes, but that part makes perfect sense to me.
> 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.
Still confusion about how a high priority translates to a number, maybe?
/*
* Priorities for the hotplug memory callback routines. Invoked from
* high to low. Higher priorities corresponds to higher numbers.
*/
This reads clear to me. I will adopt this comment if there are no additional objections.
Feel free to add
Acked-by: David Hildenbrand <david@xxxxxxxxxx>
--
Cheers
David / dhildenb
