On 8/11/2026 9:00 PM, Steven Rostedt wrote: > This is still way too verbose. Is this AI written? If so, AI is *not* your > friend. This commit was written by me, not by AI. I apologize for wasting your time. Thank you for rewriting the change. > Why are the priorities of the notifiers important here? I probably wanted to describe the entire process, so I've gone on to say too much. > Less is more when it comes to describing a bug. Got it.
> On Tue, 11 Aug 2026 14:00:05 +0800 > Michael Wu <[email protected]> wrote: > >>> What does the above mean? Are you loading two modules at the same time? >> Two modules (A and B) are loaded simultaneously on different CPUs. On the >> arm64, >> when CPU0's trace_module_notify [pri=1] and CPU1's trace_module_notify >> [pri=0] >> simultaneously perform operations on call_A, because they are in different >> cache lines, >> CPU1 may observe WRITE_ONCE(head->next, &f->link) in step (4) before >> f->link.next=next in step (2). >> At this time, CPU1 reads an uninitialized f->link.next and performs an >> operation that causes to crash. > > This is still way too verbose. Is this AI written? If so, AI is *not* your > friend. > > >> >>> What does "pri=X notifier" mean? What function calls are these coming from? >>> >> `pri=X notifier` represents `trace_events.c:trace_module_notify [pri=1]` and >> `trace.c:trace_module_notify [pri=0]`, respectively. > > Why are the priorities of the notifiers important here? > > I honestly didn't know one was allowed to load two modules at the same time > and thought that it the module logic would prevent that. But if that's not > the case, then yeah, we need protection. > > >> CPU0 (loads module A) CPU1 (loads module B) >> =============================== =============================== >> load_module(A) load_module(B) >> blocking_notifier_call_chain_robust >> blocking_notifier_call_chain_robust >> notifier_call_chain notifier_call_chain >> nb = trace_events.c: nb = trace.c: >> trace_module_notify [pri=1] trace_module_notify >> [pri=0] >> mutex_lock(&event_mutex) >> trace_event_update_all() >> trace_module_add_events(A) >> down_write(&trace_event_sem) >> __register_event(call_A) >> __add_event_to_tracers(call_A) >> event_define_fields(call_A) >> for each f: >> f = kmem_cache_alloc() >> list_add(&f->link, >> &class->fields) >> f->link.next=next; (2) >> WRITE_ONCE(head->next, >> &f->link); (4) >> update_event_fields(call_A) >> >> mutex_unlock(&event_mutex) >> list_for_each_entry(field, >> &class->fields, >> link) >> field = >> class->fields->next >> = &f->link >> = f >> (offset 0) >> >> up_write(&trace_event_sem) > > Basically this can be summed up to being: > > CPU0 (loads module A) CPU1 (loads module B) > =============================== =============================== > load_module(A) load_module(B) > notifier_call_chain notifier_call_chain > trace_module_notify trace_module_notify > mutex_lock(&event_mutex) trace_event_update_all() > trace_module_add_events(A) > down_write(&trace_event_sem) > __register_event(call_A) > __add_event_to_tracers(call_A) > event_define_fields(call_A) > for each f: > list_for_each_entry(field, > list_add(&f->link, > &class->fields, link) > &class->fields) field = > class->fields->next; > > Where you can see that one is being read while the other is being written > to. You do not need to go into details of the cache visibility here because > this is an obvious race condition. All that information just distracts from > the real issue that is being fixed. > > Less is more when it comes to describing a bug. > > I'll rewrite you change log and take the patch. > > Thanks, > > -- Steve -- Regards, Michael Wu
