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
