Re: [PATCH] nfsd: Fix build error

[Chuck Lever]

> On Mar 4, 2020, at 10:46 PM, Yuehaibing <yuehaibing@xxxxxxxxxx> wrote:
> 
> On 2020/3/5 4:06, Bruce Fields wrote:
>> On Wed, Mar 04, 2020 at 01:00:12PM -0500, Chuck Lever wrote:
>>> Hi-
>>> 
>>>> On Mar 4, 2020, at 8:18 AM, YueHaibing <yuehaibing@xxxxxxxxxx> wrote:
>>>> 
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
>>>> nfs4proc.c:(.text+0x23b7): undefined reference to `nfs42_ssc_close'
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
>>>> nfs4proc.c:(.text+0x5d2a): undefined reference to `nfs_sb_deactive'
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
>>>> nfs4proc.c:(.text+0x61d5): undefined reference to `nfs42_ssc_open'
>>>> nfs4proc.c:(.text+0x6389): undefined reference to `nfs_sb_deactive'
>>>> 
>>>> Add dependency to NFSD_V4_2_INTER_SSC to fix this.
>>>> 
>>>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>>>> Signed-off-by: YueHaibing <yuehaibing@xxxxxxxxxx>
>>>> ---
>>>> fs/nfsd/Kconfig | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>> 
>>>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
>>>> index f368f32..fc587a5 100644
>>>> --- a/fs/nfsd/Kconfig
>>>> +++ b/fs/nfsd/Kconfig
>>>> @@ -136,6 +136,7 @@ config NFSD_FLEXFILELAYOUT
>>>> 
>>>> config NFSD_V4_2_INTER_SSC
>>>> 	bool "NFSv4.2 inter server to server COPY"
>>>> +	depends on !(NFSD=y && NFS_FS=m)
>>> 
>>> The new dependency is not especially clear to me; more explanation
>>> in the patch description about the cause of the build failure
>>> would definitely be helpful.
>>> 
>>> NFSD_V4 can't be set unless NFSD is also set.
>>> 
>>> NFS_V4_2 can't be set unless NFS_V4_1 is also set, and that cannot
>>> be set unless NFS_FS is also set.
>>> 
>>> So what's really going on here?
>> 
>> I don't understand that "depends" either.
>> 
>> The fundamental problem, though, is that nfsd is calling nfs code
>> directly.
> 
> Yes
> 
>> 
>> Which I noticed in earlier review and then forgot to follow up on,
>> sorry.
>> 
>> So either we:
>> 
>> 	- let nfsd depend on nfs, fix up Kconfig to reflect the fact, or
> 
> It only fails while NFSD=y && NFS_FS=m, other cases works fine as Chuck Lever pointed.

TL;DR - we probably need both of Bruce's options, IMO.

IIUC current distributions have the NFS client and server built as
separate modules. On an NFS server, most of the time there would be
no NFS mounts, thus the NFS client module is unlikely to be in memory
when an inter-SSC type request arrives.

Stated differently, enabling SSC cannot break the kernel build when
the client is built as a module. So we need NFS_FS=m to work because
it is the most common configuration.

If creating that fix will take longer than the next merge window, it
would be good practice IMO if some kind of Kconfig dependency were
introduced now to prevent SSC from being enabled when NFS_FS=m. Someone
with better Kconfig fu than me should have a look at Yue's proposed
patch. Once the long term solution is ready, this temporary fix can
be reverted.


>> 	- write some code so nfsd can load nfs and find those symbols at
>> 	  runtime if it needs to do a copy.
>> 
>> The latter's certainly doable, but it'd be simplest to do the former.
>> Are there actually a lot of people who want nfsd but not nfs?  Does that
>> cause a real problem for anyone?
>> 
>> --b.

--
Chuck Lever