[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y2kwN5TRsAKTZmsN@casper.infradead.org>
Date: Mon, 7 Nov 2022 16:20:07 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Oded Gabbay <ogabbay@...nel.org>
Cc: MichaĆ Winiarski <michal.winiarski@...el.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
intel-gfx@...ts.freedesktop.org, David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Simon Ser <contact@...rsion.fr>
Subject: Re: [PATCH v5 1/3] drm: Use XArray instead of IDR for minors
On Sun, Nov 06, 2022 at 04:51:39PM +0200, Oded Gabbay wrote:
> I tried executing the following code in a dummy driver I wrote:
You don't need to write a dummy driver; you can write test-cases
entirely in userspace. Just add them to lib/test_xarray.c and then
make -C tools/testing/radix-tree/
> static DEFINE_XARRAY_ALLOC(xa_dummy);
> void check_xa(void *pdev)
> {
> void *entry;
> int ret, index;
>
> ret = xa_alloc(&xa_dummy, &index, NULL, XA_LIMIT(0, 63), GFP_NOWAIT);
> if (ret < 0)
> return ret;
>
> entry = xa_cmpxchg(&xa_dummy, index, NULL, pdev, GFP_KERNEL);
> if (xa_is_err(entry))
> return;
>
> xa_lock(&xa_dummy);
> xa_dev = xa_load(&xa_dummy, index);
> xa_unlock(&xa_dummy);
> }
>
> And to my surprise xa_dev is always NULL, when it should be pdev.
> I believe that because we first allocate the entry with NULL, it is
> considered a "zero" entry in the XA.
> And when we replace it, this attribute doesn't change so when we do
> xa_load, the xa code thinks the entry is a "zero" entry and returns
> NULL.
There's no "attribute" to mark a zero entry. It's just a zero entry.
The problem is that you're cmpxchg'ing against NULL, and it's not NULL,
it's the ZERO entry. This is even documented in the test code:
/* cmpxchg sees a reserved entry as ZERO */
XA_BUG_ON(xa, xa_reserve(xa, 12345678, GFP_KERNEL) != 0);
XA_BUG_ON(xa, xa_cmpxchg(xa, 12345678, XA_ZERO_ENTRY,
xa_mk_value(12345678), GFP_NOWAIT) != NULL);
I'm not quite sure why you're using xa_cmpxchg() here anyway; if you
allocated it, it's yours and you can just xa_store() to it.
Powered by blists - more mailing lists