| Message ID | 163174721123.350433.6338166230233894732.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: sync libxfs with 5.14 | expand |
On Wed, Sep 15, 2021 at 04:06:51PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Create a mockups.h for mocked-up versions of kernel data structures to > ease porting of libxfs code. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > include/libxfs.h | 1 + > libfrog/Makefile | 1 + > libfrog/mockups.h | 19 +++++++++++++++++++ > libxfs/libxfs_priv.h | 4 +--- > 4 files changed, 22 insertions(+), 3 deletions(-) I don't really like moving this stuff to libfrog. The whole point of libxfs/libxfs_priv.h is to define the kernel wrapper stuff that libxfs needs to compile and should never be seen by anything outside libxfs/... Indeed, we -cannot- use spinlocks in userspace code, so I really don't see why we'd want to make them more widely visible to the userspace xfsprogs code... Cheers, Dave.
On Thu, Sep 16, 2021 at 10:46:46AM +1000, Dave Chinner wrote: > On Wed, Sep 15, 2021 at 04:06:51PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Create a mockups.h for mocked-up versions of kernel data structures to > > ease porting of libxfs code. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > include/libxfs.h | 1 + > > libfrog/Makefile | 1 + > > libfrog/mockups.h | 19 +++++++++++++++++++ > > libxfs/libxfs_priv.h | 4 +--- > > 4 files changed, 22 insertions(+), 3 deletions(-) > > I don't really like moving this stuff to libfrog. The whole point of > libxfs/libxfs_priv.h is to define the kernel wrapper stuff that > libxfs needs to compile and should never be seen by anything outside > libxfs/... How did you handle this in your xfsprogs port? I /think/ the only reason we need the mockups is to handle the perag structure in xfs_ag.h? In that case, I guess one could simply omit the stuff below the "kernel only structures below this line" line? In that case, can you (or anyone, really) fix libxfs-compare to be smart enough to filter out the "#ifdef __KERNEL__" parts of libxfs from the diff? --D > Indeed, we -cannot- use spinlocks in userspace code, so I really > don't see why we'd want to make them more widely visible to the > userspace xfsprogs code... > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
On Wed, Sep 15, 2021 at 05:58:21PM -0700, Darrick J. Wong wrote: > On Thu, Sep 16, 2021 at 10:46:46AM +1000, Dave Chinner wrote: > > On Wed, Sep 15, 2021 at 04:06:51PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Create a mockups.h for mocked-up versions of kernel data structures to > > > ease porting of libxfs code. > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > include/libxfs.h | 1 + > > > libfrog/Makefile | 1 + > > > libfrog/mockups.h | 19 +++++++++++++++++++ > > > libxfs/libxfs_priv.h | 4 +--- > > > 4 files changed, 22 insertions(+), 3 deletions(-) > > > > I don't really like moving this stuff to libfrog. The whole point of > > libxfs/libxfs_priv.h is to define the kernel wrapper stuff that > > libxfs needs to compile and should never be seen by anything outside > > libxfs/... > > How did you handle this in your xfsprogs port? I /think/ the only > reason we need the mockups is to handle the perag structure in xfs_ag.h? > In that case, I guess one could simply omit the stuff below the "kernel > only structures below this line" line? I just put an #ifdef __KERNEL__ in the userspace code, like we have in userspace libxfs/xfs_btree.c for the btree split hand-off code. > In that case, can you (or anyone, really) fix libxfs-compare to be smart > enough to filter out the "#ifdef __KERNEL__" parts of libxfs from the > diff? You mean tools/libxfs-diff? I'm not sure that's a simple thing to do because of the #else cases that go along with define in xfs_btree.c. Is there really enough noise from libxfs-diff at the moment that this is actually a problem? As it is, my longer term plan it to actually properly support things like spinlocks, atomics, rcu, etc in xfsprogs via pthread and liburcu wrappers defined in include/<foo.h> that are xfsprogs wide. At that point, the wrappers in libxfs/libxfs_priv.h then simply disappear. I'd prefer we move towards proper support for these primitives rather than just rearranging how we mock them up... Cheers, Dave.
On Thu, Sep 16, 2021 at 11:29:16AM +1000, Dave Chinner wrote: > As it is, my longer term plan it to actually properly support things > like spinlocks, atomics, rcu, etc in xfsprogs via pthread and > liburcu wrappers defined in include/<foo.h> that are xfsprogs wide. > At that point, the wrappers in libxfs/libxfs_priv.h then simply > disappear. > > I'd prefer we move towards proper support for these primitives > rather than just rearranging how we mock them up... Just dug some patches out of a series (not up to date so probably won't apply, but...) so you can see what I'm suggesting. I'll post them as a reply here... Cheers, Dave.
On 9/15/21 7:46 PM, Dave Chinner wrote: > On Wed, Sep 15, 2021 at 04:06:51PM -0700, Darrick J. Wong wrote: >> From: Darrick J. Wong <djwong@kernel.org> >> >> Create a mockups.h for mocked-up versions of kernel data structures to >> ease porting of libxfs code. >> >> Signed-off-by: Darrick J. Wong <djwong@kernel.org> >> --- >> include/libxfs.h | 1 + >> libfrog/Makefile | 1 + >> libfrog/mockups.h | 19 +++++++++++++++++++ >> libxfs/libxfs_priv.h | 4 +--- >> 4 files changed, 22 insertions(+), 3 deletions(-) > > I don't really like moving this stuff to libfrog. The whole point of > libxfs/libxfs_priv.h is to define the kernel wrapper stuff that > libxfs needs to compile and should never be seen by anything outside > libxfs/... I had the same reaction to seeing these in libfrog/ TBH. IIRC adding this all to libxfs_priv.h caused me problems, though I don't remember exactly why. I had more luck creating a new header file in include/mockups.h, and then I had to include /that/ in both libxfs.h and libxfs_priv.h. I don't remember how I ended up like that... but without the libxfs.h include, I ended up with: In file included from ../include/libxfs.h:73:0, from topology.c:7: ../libxfs/xfs_ag.h:75:2: error: unknown type name 'spinlock_t' spinlock_t pag_state_lock; ^ I do think that more functionally-named, separate header files might be good, rather than just "dump more stuff in libxfs_priv.h" because it's getting to be quite the junk drawer. ;) But I see Dave may have a grander plan than that ;) -Eric > Indeed, we -cannot- use spinlocks in userspace code, so I really > don't see why we'd want to make them more widely visible to the > userspace xfsprogs code... > > Cheers, > > Dave. >
diff --git a/include/libxfs.h b/include/libxfs.h index 36ae86cc..c297152f 100644 --- a/include/libxfs.h +++ b/include/libxfs.h @@ -17,6 +17,7 @@ #include "bitops.h" #include "kmem.h" #include "libfrog/radix-tree.h" +#include "libfrog/mockups.h" #include "atomic.h" #include "xfs_types.h" diff --git a/libfrog/Makefile b/libfrog/Makefile index 01107082..5381d9b5 100644 --- a/libfrog/Makefile +++ b/libfrog/Makefile @@ -41,6 +41,7 @@ crc32defs.h \ crc32table.h \ fsgeom.h \ logging.h \ +mockups.h \ paths.h \ projects.h \ ptvar.h \ diff --git a/libfrog/mockups.h b/libfrog/mockups.h new file mode 100644 index 00000000..f00a9e41 --- /dev/null +++ b/libfrog/mockups.h @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2000-2005 Silicon Graphics, Inc. + * All Rights Reserved. + */ +#ifndef __LIBFROG_MOCKUPS_H__ +#define __LIBFROG_MOCKUPS_H__ + +/* Mockups of kernel data structures. */ + +typedef struct spinlock { +} spinlock_t; + +#define spin_lock_init(lock) ((void) 0) + +#define spin_lock(a) ((void) 0) +#define spin_unlock(a) ((void) 0) + +#endif /* __LIBFROG_MOCKUPS_H__ */ diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h index 7181a858..727f6be8 100644 --- a/libxfs/libxfs_priv.h +++ b/libxfs/libxfs_priv.h @@ -47,6 +47,7 @@ #include "bitops.h" #include "kmem.h" #include "libfrog/radix-tree.h" +#include "libfrog/mockups.h" #include "atomic.h" #include "xfs_types.h" @@ -205,9 +206,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC }; #endif /* miscellaneous kernel routines not in user space */ -#define spin_lock_init(a) ((void) 0) -#define spin_lock(a) ((void) 0) -#define spin_unlock(a) ((void) 0) #define likely(x) (x) #define unlikely(x) (x) #define rcu_read_lock() ((void) 0)