On Wed 05-02-20 14:45:12, Jason Gunthorpe wrote: > On Tue, Feb 04, 2020 at 03:25:09PM +0100, Jan Kara wrote: > > __xa_cmpxchg() relies on xas_store() to set XA_FREE_MARK when storing > > NULL into xarray that has free tracking enabled. Make the setting of > > XA_FREE_MARK explicit similarly as its clearing currently it. > > > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > lib/xarray.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/lib/xarray.c b/lib/xarray.c > > index ae8b7070e82c..4e32497c51bd 100644 > > +++ b/lib/xarray.c > > @@ -1477,8 +1477,12 @@ void *__xa_cmpxchg(struct xarray *xa, unsigned long index, > > curr = xas_load(&xas); > > if (curr == old) { > > xas_store(&xas, entry); > > - if (xa_track_free(xa) && entry && !curr) > > - xas_clear_mark(&xas, XA_FREE_MARK); > > + if (xa_track_free(xa)) { > > + if (entry && !curr) > > + xas_clear_mark(&xas, XA_FREE_MARK); > > + else if (!entry && curr) > > + xas_set_mark(&xas, XA_FREE_MARK); > > + } > > This feels like an optimization that should also happen for > __xa_store, which has very similar code: > > curr = xas_store(&xas, entry); > if (xa_track_free(xa)) > xas_clear_mark(&xas, XA_FREE_MARK); > > Something like > > if (xa_track_free(xa) && entry && !curr) > xas_clear_mark(&xas, XA_FREE_MARK); > > ? Yeah, entry != NULL is guaranteed for __xa_store() (see how it transforms NULL to XA_ZERO_ENTRY a few lines above) but !curr is probably a good condition to add to save some unnecessary clearing when overwriting existing values. It is unrelated to this patch though, just a separate optimization so I'll add that as a separate patch to the series. Thanks for the idea. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR