Re: [PATCH v9 07/61] xarray: Add the xa_lock to the radix_tree_root
On Thu, Apr 12, 2018 at 3:22 PM, Matthew Wilcox wrote: > On Thu, Apr 12, 2018 at 03:16:23PM -0600, Ross Zwisler wrote: >> On Thu, Apr 12, 2018 at 3:10 PM, Matthew Wilcox wrote: >> > On Thu, Apr 12, 2018 at 02:59:32PM -0600, Ross Zwisler wrote: >> >> This is causing build breakage in the radix tree test suite in the >> >> current linux/master: >> >> >> >> ./linux/../../../../include/linux/idr.h: In function ‘idr_init_base’: >> >> ./linux/../../../../include/linux/radix-tree.h:129:2: warning: >> >> implicit declaration of function ‘spin_lock_init’; did you mean >> >> ‘spinlock_t’? [-Wimplicit-function-declaration] >> > >> > Argh. That was added two patches later in >> > "xarray: Add definition of struct xarray": >> > >> > diff --git a/tools/include/linux/spinlock.h >> > b/tools/include/linux/spinlock.h >> > index b21b586b9854..4ec4d2cbe27a 100644 >> > --- a/tools/include/linux/spinlock.h >> > +++ b/tools/include/linux/spinlock.h >> > @@ -6,8 +6,9 @@ >> > #include >> > >> > #define spinlock_t pthread_mutex_t >> > -#define DEFINE_SPINLOCK(x) pthread_mutex_t x = >> > PTHREAD_MUTEX_INITIALIZER; >> > +#define DEFINE_SPINLOCK(x) pthread_mutex_t x = >> > PTHREAD_MUTEX_INITIALIZER >> > #define __SPIN_LOCK_UNLOCKED(x) >> > (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER >> > +#define spin_lock_init(x) pthread_mutex_init(x, NULL) >> > >> > #define spin_lock_irqsave(x, f)(void)f, >> > pthread_mutex_lock(x) >> > #define spin_unlock_irqrestore(x, f) (void)f, pthread_mutex_unlock(x) >> > >> > I didn't pick up that it was needed this early on in the patch series. >> >> Hmmm..I don't know if it's a patch ordering issue, because this >> happens with the current linux/master where presumably all the patches >> are present? > > No, Andrew only merged the first 8 or so because of lack of review of > the remaining patches. Even though I cc'd people as hard as I could. > Including you. :-P > > You could, for example, review the DAX patches ... Fair enough. Let's get the radix tree working, and in the mean time I'll throw it into my xfstests testing setup & take a look at the DAX patches.
Re: [PATCH v9 07/61] xarray: Add the xa_lock to the radix_tree_root
On Thu, Apr 12, 2018 at 03:16:23PM -0600, Ross Zwisler wrote: > On Thu, Apr 12, 2018 at 3:10 PM, Matthew Wilcox wrote: > > On Thu, Apr 12, 2018 at 02:59:32PM -0600, Ross Zwisler wrote: > >> This is causing build breakage in the radix tree test suite in the > >> current linux/master: > >> > >> ./linux/../../../../include/linux/idr.h: In function ‘idr_init_base’: > >> ./linux/../../../../include/linux/radix-tree.h:129:2: warning: > >> implicit declaration of function ‘spin_lock_init’; did you mean > >> ‘spinlock_t’? [-Wimplicit-function-declaration] > > > > Argh. That was added two patches later in > > "xarray: Add definition of struct xarray": > > > > diff --git a/tools/include/linux/spinlock.h b/tools/include/linux/spinlock.h > > index b21b586b9854..4ec4d2cbe27a 100644 > > --- a/tools/include/linux/spinlock.h > > +++ b/tools/include/linux/spinlock.h > > @@ -6,8 +6,9 @@ > > #include > > > > #define spinlock_t pthread_mutex_t > > -#define DEFINE_SPINLOCK(x) pthread_mutex_t x = > > PTHREAD_MUTEX_INITIALIZER; > > +#define DEFINE_SPINLOCK(x) pthread_mutex_t x = > > PTHREAD_MUTEX_INITIALIZER > > #define __SPIN_LOCK_UNLOCKED(x) > > (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER > > +#define spin_lock_init(x) pthread_mutex_init(x, NULL) > > > > #define spin_lock_irqsave(x, f)(void)f, > > pthread_mutex_lock(x) > > #define spin_unlock_irqrestore(x, f) (void)f, pthread_mutex_unlock(x) > > > > I didn't pick up that it was needed this early on in the patch series. > > Hmmm..I don't know if it's a patch ordering issue, because this > happens with the current linux/master where presumably all the patches > are present? No, Andrew only merged the first 8 or so because of lack of review of the remaining patches. Even though I cc'd people as hard as I could. Including you. :-P You could, for example, review the DAX patches ...
Re: [PATCH v9 07/61] xarray: Add the xa_lock to the radix_tree_root
On Thu, Apr 12, 2018 at 3:10 PM, Matthew Wilcox wrote: > On Thu, Apr 12, 2018 at 02:59:32PM -0600, Ross Zwisler wrote: >> This is causing build breakage in the radix tree test suite in the >> current linux/master: >> >> ./linux/../../../../include/linux/idr.h: In function ‘idr_init_base’: >> ./linux/../../../../include/linux/radix-tree.h:129:2: warning: >> implicit declaration of function ‘spin_lock_init’; did you mean >> ‘spinlock_t’? [-Wimplicit-function-declaration] > > Argh. That was added two patches later in > "xarray: Add definition of struct xarray": > > diff --git a/tools/include/linux/spinlock.h b/tools/include/linux/spinlock.h > index b21b586b9854..4ec4d2cbe27a 100644 > --- a/tools/include/linux/spinlock.h > +++ b/tools/include/linux/spinlock.h > @@ -6,8 +6,9 @@ > #include > > #define spinlock_t pthread_mutex_t > -#define DEFINE_SPINLOCK(x) pthread_mutex_t x = PTHREAD_MUTEX_INITIALIZER; > +#define DEFINE_SPINLOCK(x) pthread_mutex_t x = PTHREAD_MUTEX_INITIALIZER > #define __SPIN_LOCK_UNLOCKED(x) > (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER > +#define spin_lock_init(x) pthread_mutex_init(x, NULL) > > #define spin_lock_irqsave(x, f)(void)f, pthread_mutex_lock(x) > #define spin_unlock_irqrestore(x, f) (void)f, pthread_mutex_unlock(x) > > I didn't pick up that it was needed this early on in the patch series. Hmmm..I don't know if it's a patch ordering issue, because this happens with the current linux/master where presumably all the patches are present?
Re: [PATCH v9 07/61] xarray: Add the xa_lock to the radix_tree_root
On Thu, Apr 12, 2018 at 02:59:32PM -0600, Ross Zwisler wrote: > This is causing build breakage in the radix tree test suite in the > current linux/master: > > ./linux/../../../../include/linux/idr.h: In function ‘idr_init_base’: > ./linux/../../../../include/linux/radix-tree.h:129:2: warning: > implicit declaration of function ‘spin_lock_init’; did you mean > ‘spinlock_t’? [-Wimplicit-function-declaration] Argh. That was added two patches later in "xarray: Add definition of struct xarray": diff --git a/tools/include/linux/spinlock.h b/tools/include/linux/spinlock.h index b21b586b9854..4ec4d2cbe27a 100644 --- a/tools/include/linux/spinlock.h +++ b/tools/include/linux/spinlock.h @@ -6,8 +6,9 @@ #include #define spinlock_t pthread_mutex_t -#define DEFINE_SPINLOCK(x) pthread_mutex_t x = PTHREAD_MUTEX_INITIALIZER; +#define DEFINE_SPINLOCK(x) pthread_mutex_t x = PTHREAD_MUTEX_INITIALIZER #define __SPIN_LOCK_UNLOCKED(x) (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER +#define spin_lock_init(x) pthread_mutex_init(x, NULL) #define spin_lock_irqsave(x, f)(void)f, pthread_mutex_lock(x) #define spin_unlock_irqrestore(x, f) (void)f, pthread_mutex_unlock(x) I didn't pick up that it was needed this early on in the patch series.
Re: [PATCH v9 07/61] xarray: Add the xa_lock to the radix_tree_root
On Tue, Mar 13, 2018 at 7:25 AM, Matthew Wilcox wrote: > From: Matthew Wilcox > > This results in no change in structure size on 64-bit machines as it > fits in the padding between the gfp_t and the void *. 32-bit machines > will grow the structure from 8 to 12 bytes. Almost all radix trees > are protected with (at least) a spinlock, so as they are converted from > radix trees to xarrays, the data structures will shrink again. > > Initialising the spinlock requires a name for the benefit of lockdep, > so RADIX_TREE_INIT() now needs to know the name of the radix tree it's > initialising, and so do IDR_INIT() and IDA_INIT(). > > Also add the xa_lock() and xa_unlock() family of wrappers to make it > easier to use the lock. If we could rely on -fplan9-extensions in > the compiler, we could avoid all of this syntactic sugar, but that > wasn't added until gcc 4.6. > > Signed-off-by: Matthew Wilcox > Reviewed-by: Jeff Layton This is causing build breakage in the radix tree test suite in the current linux/master: 5d1365940a68 (linux/master) Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net Here's the first breakage: cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr.o idr.c In file included from ./linux/radix-tree.h:6:0, from ./linux/../../../../include/linux/idr.h:15, from ./linux/idr.h:1, from idr.c:4: ./linux/../../../../include/linux/idr.h: In function ‘idr_init_base’: ./linux/../../../../include/linux/radix-tree.h:129:2: warning: implicit declaration of function ‘spin_lock_init’; did you mean ‘spinlock_t’? [-Wimplicit-function-declaration] spin_lock_init(&(root)->xa_lock);\ ^ ./linux/../../../../include/linux/idr.h:126:2: note: in expansion of macro ‘INIT_RADIX_TREE’ INIT_RADIX_TREE(&idr->idr_rt, IDR_RT_MARKER); ^~~
