| Message ID | 20231026192830.21288-1-rdunlap@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | exportfs: handle CONFIG_EXPORTFS=m also | expand |
On Thu, Oct 26, 2023 at 10:28 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > When CONFIG_EXPORTFS=m, there are multiple build errors due to > the header <linux/exportfs.h> not handling the =m setting correctly. > Change the header file to check for CONFIG_EXPORTFS enabled at all > instead of just set =y. > > Fixes: dfaf653dc415 ("exportfs: make ->encode_fh() a mandatory method for NFS export") > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Chuck Lever <chuck.lever@oracle.com> > Cc: Jeff Layton <jlayton@kernel.org> > Cc: linux-nfs@vger.kernel.org > Cc: Amir Goldstein <amir73il@gmail.com> > Cc: Christian Brauner <brauner@kernel.org> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > Cc: linux-fsdevel@vger.kernel.org > > --- > include/linux/exportfs.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff -- a/include/linux/exportfs.h b/include/linux/exportfs.h > --- a/include/linux/exportfs.h > +++ b/include/linux/exportfs.h > @@ -314,7 +314,7 @@ extern struct dentry *exportfs_decode_fh > /* > * Generic helpers for filesystems. > */ > -#ifdef CONFIG_EXPORTFS > +#if IS_ENABLED(CONFIG_EXPORTFS) > int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > struct inode *parent); > #else Thanks for the fix, but Arnd reported that this fix could cause a link issue in some configuration - I did not verify. I would much rather turn EXPORTFS into a bool config and avoid the unneeded build test matrix. See comparison to the amount of code of EXPORTFS to BUFFER_HEAD and FS_IOMAP code which are bool: ~/src/linux$ wc -l fs/exportfs/*.c 636 fs/exportfs/expfs.c ~/src/linux$ wc -l fs/buffer.c fs/mpage.c 3164 fs/buffer.c 685 fs/mpage.c 3849 total ~/src/linux$ wc -l fs/iomap/*.c 2002 fs/iomap/buffered-io.c 754 fs/iomap/direct-io.c 124 fs/iomap/fiemap.c 97 fs/iomap/iter.c 104 fs/iomap/seek.c 195 fs/iomap/swapfile.c 13 fs/iomap/trace.c 3289 total Thanks, Amir.
On Thu, Oct 26, 2023 at 10:46:06PM +0300, Amir Goldstein wrote: > I would much rather turn EXPORTFS into a bool config > and avoid the unneeded build test matrix. Yes. Especially given that the defaul on open by handle syscalls require it anyway.
On Fri, Oct 27, 2023 at 9:01 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Thu, Oct 26, 2023 at 10:46:06PM +0300, Amir Goldstein wrote: > > I would much rather turn EXPORTFS into a bool config > > and avoid the unneeded build test matrix. > > Yes. Especially given that the defaul on open by handle syscalls > require it anyway. Note that those syscalls depend on CONFIG_FHANDLE and the latter selects EXPORTFS. Nevertheless, the EXPORTFS=m config seems useless. I will send a patch to change it. The bigger issue is that so many of the filesystems that use the generic export ops do not select EXPORTFS, so it's easier to leave the generic helper in libfs.c as Arnd suggested. Thanks, Amir.
On Fri, Oct 27, 2023 at 09:11:57AM +0300, Amir Goldstein wrote: > On Fri, Oct 27, 2023 at 9:01 AM Christoph Hellwig <hch@infradead.org> wrote: > > > > On Thu, Oct 26, 2023 at 10:46:06PM +0300, Amir Goldstein wrote: > > > I would much rather turn EXPORTFS into a bool config > > > and avoid the unneeded build test matrix. > > > > Yes. Especially given that the defaul on open by handle syscalls > > require it anyway. > > Note that those syscalls depend on CONFIG_FHANDLE and the latter > selects EXPORTFS. Yes, this means that for all somewhat sane configfs exportfs if always built in anyway. And for the ones where it isn't because people are concerned about micro-optimizing kernel size, nfsd is unlikely to be built in either. > The bigger issue is that so many of the filesystems that use the > generic export ops do not select EXPORTFS, so it's easier to > leave the generic helper in libfs.c as Arnd suggested. Agreed.
diff -- a/include/linux/exportfs.h b/include/linux/exportfs.h --- a/include/linux/exportfs.h +++ b/include/linux/exportfs.h @@ -314,7 +314,7 @@ extern struct dentry *exportfs_decode_fh /* * Generic helpers for filesystems. */ -#ifdef CONFIG_EXPORTFS +#if IS_ENABLED(CONFIG_EXPORTFS) int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, struct inode *parent); #else
When CONFIG_EXPORTFS=m, there are multiple build errors due to the header <linux/exportfs.h> not handling the =m setting correctly. Change the header file to check for CONFIG_EXPORTFS enabled at all instead of just set =y. Fixes: dfaf653dc415 ("exportfs: make ->encode_fh() a mandatory method for NFS export") Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Chuck Lever <chuck.lever@oracle.com> Cc: Jeff Layton <jlayton@kernel.org> Cc: linux-nfs@vger.kernel.org Cc: Amir Goldstein <amir73il@gmail.com> Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org --- include/linux/exportfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)