| Message ID | 20200304131803.46560-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: Fix build error | expand |
Hi- > On Mar 4, 2020, at 8:18 AM, YueHaibing <yuehaibing@huawei.com> 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@huawei.com> > --- > 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? > depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 > help > This option enables support for NFSv4.2 inter server to > -- > 2.7.4 > > -- Chuck Lever
On Wed, Mar 04, 2020 at 01:00:12PM -0500, Chuck Lever wrote: > Hi- > > > On Mar 4, 2020, at 8:18 AM, YueHaibing <yuehaibing@huawei.com> 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@huawei.com> > > --- > > 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. 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 - 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.
> On Mar 4, 2020, at 3:06 PM, Bruce Fields <bfields@fieldses.org> 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@huawei.com> 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@huawei.com> >>> --- >>> 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. > > 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 > - write some code so nfsd can load nfs and find those symbols at > runtime if it needs to do a copy. Another practical option would be to create a copy of these functions in the server's SSC code. At least nfs_sb_deactive() is not large. Not clear if nfs42_ssc_open/close are even used on the client. Maybe they could be moved to the server? > 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
On 3/4/20, 4:05 PM, "Chuck Lever" <chuck.lever@oracle.com> wrote:
NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> On Mar 4, 2020, at 3:06 PM, Bruce Fields <bfields@fieldses.org> 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@huawei.com> 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@huawei.com>
>>> ---
>>> 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.
>
> 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
> - write some code so nfsd can load nfs and find those symbols at
> runtime if it needs to do a copy.
Another practical option would be to create a copy of these functions in
the server's SSC code. At least nfs_sb_deactive() is not large.
Not clear if nfs42_ssc_open/close are even used on the client. Maybe they
could be moved to the server?
Nfs42_ssc_open/close are not used on the client but their insides use client's structures (like nfs_fh, nfs4_stateid, etc) and calls nfs4_proc_getattr, nfs_fhget and others and VFS functions. It'll be a lot to 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
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@huawei.com> 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@huawei.com> >>> --- >>> 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. > - 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. > > . >
> On Mar 4, 2020, at 10:46 PM, Yuehaibing <yuehaibing@huawei.com> 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@huawei.com> 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@huawei.com> >>>> --- >>>> 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
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) depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 help This option enables support for NFSv4.2 inter server to
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@huawei.com> --- fs/nfsd/Kconfig | 1 + 1 file changed, 1 insertion(+)