2023-10-26 19:28:52

by Randy Dunlap

[permalink] [raw]
Subject: [PATCH] exportfs: handle CONFIG_EXPORTFS=m also

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 <[email protected]>
Cc: Chuck Lever <[email protected]>
Cc: Jeff Layton <[email protected]>
Cc: [email protected]
Cc: Amir Goldstein <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: [email protected]

---
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


2023-10-26 19:46:25

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH] exportfs: handle CONFIG_EXPORTFS=m also

On Thu, Oct 26, 2023 at 10:28 PM Randy Dunlap <[email protected]> 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 <[email protected]>
> Cc: Chuck Lever <[email protected]>
> Cc: Jeff Layton <[email protected]>
> Cc: [email protected]
> Cc: Amir Goldstein <[email protected]>
> Cc: Christian Brauner <[email protected]>
> Cc: Alexander Viro <[email protected]>
> Cc: [email protected]
>
> ---
> 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.

2023-10-27 06:12:17

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH] exportfs: handle CONFIG_EXPORTFS=m also

On Fri, Oct 27, 2023 at 9:01 AM Christoph Hellwig <[email protected]> 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.