Re: [PATCH 0/2] maple_tree: fix maple_range_64 crashes in RCU mode

[Liam R. Howlett]

On 26/09/15 11:23AM, Liam R. Howlett wrote:
> On 26/09/13 03:26PM, Dimitris Charisis wrote:
> > I was running a patched kernel using a maple tree in RCU mode without
> > specifying the MT_FLAGS_ALLOC_RANGE flag and the kernel crashed. I
> > switched to an upstream kernel, and after some investigation and
> > debugging (thanks tools/testing/radix-tree/maple.c) I discovered that
> > there were two separate issues on the RCU destroy path that were
> > responsible. Both arising from the fact that a maple_range_64 node that
> > is not full repurposes its last slot to hold metadata. Each issue is
> > addressed by one of the two patches.
> > 
> > AFAIK there is no in-tree user of maple_range_64 nodes in RCU mode at
> > the moment. All in-tree users of MT_FLAGS_USE_RCU that I found also set
> > MT_FLAGS_ALLOC_RANGE which makes the internal nodes maple_arange_64.
> 
> Hello!
> 
> Thank you for your investigation and patches.  I will have a closer look
> soon, but for now I have a request for this series.
> 
> > 
> > Below is the minimal reproducer I used during debugging.
> > 
> > static DEFINE_MTREE(tree);
> > static int __init maple_fix_init(void)
> > {
> >         unsigned long i, j;
> > 
> > 	for (i = 10; i < 4000; i++) {
> >                 mt_init_flags(&tree, MT_FLAGS_USE_RCU);
> > 
> >                 for (j = 0; j < i; j++) {
> >                         mtree_insert_range(&tree, j * 10, j * 10 + 9,
> >                                         xa_mk_value(j), GFP_KERNEL);
> >                 }
> > 
> >                 mtree_destroy(&tree);
> >                 rcu_barrier();
> > 	}
> > 	return 0;
> > }
> 
> Awesome!  What I ask people to do is to create a patch for the
> tools/testing/radix-tree/maple.c with the reproducer in its own patch
> that gets added after the fix.  This way we never re-introduce new
> features and the testing keeps passing for every bisection.
> 
> If you have one of these for each bug, then please add them after the
> fix.  I understand rcu is tricky to test when a race is involved, but I
> do have testing for many threads (which you probably saw).
> 
> The testing is less organized than I'd like and since this is RCU
> specific, you may need to use the testing file and not the module
> testing in lib/test_maple_tree.c.  If possible, I add the testing to the
> module, but if internal poking or threads need to be used, we put them
> in the userspace testing.

Your fixes look good - Thanks again.

Please respin with a few changes:
1. The message in this email will be preserved by Andrew including it as
part of the first patch change log so "I was running a patched kernel.."
should probably be removed and made more into a change log statement.

2. Add the testing to the userspace testing patch to enhance the test
suite please.

Thanks,
Liam