On Mon, Apr 03, 2023 at 11:45:08AM -0400, Liam R. Howlett wrote: > * Mark Brown <broonie@xxxxxxxxxx> [230329 20:10]: > > + /* Any adjacent entries to extend/merge? */ > > + mas_set_range(&mas, reg - 1, reg + 1); > > + index = reg; > > + last = reg; > > + > > + lower = mas_find(&mas, reg - 1); > If you just want to check the previous, you can use: > mas_prev(&mas, reg - 1); > This will try the previous entry without rewalking from the top of the > tree and you don't need to mas_set_range() call. I did give this a spin but it doesn't seem to be doing what I'd expect for indexes 0 and 1, in a register map with index 0 if we attempt to insert index 1 the mas_prev() doesn't find the existing entry for index 0 so we don't attempt to combine them. It seems to do the right thing for non-zero indexes, I'll have a poke at some point. > > + /* > > + * This is safe because the regmap lock means the Maple lock > > + * is redundant, but we need to take it due to lockdep asserts > > + * in the maple tree code. > > + */ > > + mas_lock(&mas); > > + mas_set_range(&mas, index, last); > > + ret = mas_store_gfp(&mas, entry, GFP_KERNEL); > You can avoid this walk as well by changing the order of the code > before: > mas_walk(&mas, reg); > if entry... return > mas_next(&mas, reg + 1); > ... > mas_prev(&mas, reg - 1); > ... > This should now be pointing at the location mas_store_gfp() expects: > mas.last = last; > ret = mas_store_gfp() This appears to be triggering data corruption for me in the cache drop test, again I'll have a poke at some point. We seem to be getting rubbish for the upper data block, though only in the drop test which means it's possibly an interaction with how the tree is affected by dropping the middle of a block. Might well be something stupid I'm doing either here or in the drop function.
Attachment:
signature.asc
Description: PGP signature
