Re: [PATCH] maple_tree: Annotate lockless pivot reads for KCSAN

["Hui Zhu" <hui.zhu@linux.dev>]

> 
> On 26/08/26 09:41AM, Andrew Morton wrote:
> 
> > 
> > On Wed, 26 Aug 2026 15:44:30 +0800 "Hui Zhu" <hui.zhu@linux.dev> wrote:
> >  
> >  In RCU mode, replaced maple nodes are marked dead and freed via RCU
> >  after the new node has been published. Arming the RCU free writes
> >  node->rcu.next and node->rcu.func, which share storage with
> >  pivot[0] and pivot[1] (see struct maple_node), while lockless
> >  readers may still walk the dead node. These stores therefore race
> >  with the pivot loads performed by the walkers.
> >  
> >  This is harmless: the writer marks the node dead with an smp_wmb()
> >  before arming the rcu_head, and the walkers re-check ma_dead_node()
> >  after reading the node and restart the walk when the node is dead,
> >  so any pivot read that raced with the rcu_head stores is discarded.
> >  KCSAN cannot see this protocol and reports the plain accesses, so
> >  annotate the lockless pivot reads with data_race() through a new
> >  ma_pivot_rcu() helper.
> >  
> >  Found by fuzzing on a 6.6 kernel; the race still exists on
> >  mainline. No functional change intended.
> >  
> >  
> >  Thanks. AI review suggests that the patch is correct, but incomplete?
> >  
> >  https://sashiko.dev/#/patchset/20260826074430.1139325-1-hui.zhu@linux.dev
> > 
> Yes, this is literally every read of numerous pivots on any reader that
> would need something like this. Most code is shared with the writer
> side, so we'll have data_race() annotation where it is not needed there.
> 
> I don't like the name of the function and I don't agree that it is worth
> doing, especially half of it. If you notice ma_ functions take a maple
> node as the first argument, but your new function takes an array pointer
> in the node.
> 
> The from address does not agree with the sign-off on the patch.
> 
> I also don't think a benign race needs a Fixes tag?
> 
> It might be worth looking at other ways to calm kcsans down such as the
> type qualifier __data_racy, or maybe the makefile option.

Hi Liam,

__data_racy is defined as volatile for KCSAN kernels, so the qualifier
has to propagate to every access site: ma_pivots() would return a
volatile pointer, and all the "unsigned long *pivots" locals and helper
parameters in maple_tree.c (about 25 sites) would need the qualifier
too.
It also marks the whole pivot array as racy, while only pivot[0] and
pivot[1] actually overlap the rcu_head.

The Makefile option (KCSAN_SANITIZE_maple_tree.o := n) is a one-liner,
but it disables KCSAN for the entire file, so any real data race
introduced in maple_tree.c later would go unnoticed.

Do you think one of these two is still the better choice, or should I
keep the current approach and fix the patch according to your review
comments?

Best,
Hui

> 
> Thanks,
> Liam
>