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.
