linux-next: build failure after merge of the nfsd tree

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

Hi J.,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]

Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
RPCGSS authentication").  gss_mech_get_by_OID() made static to
net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
tree that you did not merge).

I don't know how to fix this, so I have used the nfsd tree from
next-20130426 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi J.,
> 
> After merging the nfsd tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> 
> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> RPCGSS authentication").  gss_mech_get_by_OID() made static to
> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> tree that you did not merge).
> 
> I don't know how to fix this, so I have used the nfsd tree from
> next-20130426 for today.

Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.


-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> 
> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi J.,
> > 
> > After merging the nfsd tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> > 
> > net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> > net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> > 
> > Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> > RPCGSS authentication").  gss_mech_get_by_OID() made static to
> > net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> > Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> > tree that you did not merge).
> > 
> > I don't know how to fix this, so I have used the nfsd tree from
> > next-20130426 for today.
> 
> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.

I'm happy to take those patches whenever you consider them ready.  Would
that fix the problem?

Also: it looks like 030d794bf498 "SUNRPC: Introduce
rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
of *that*?

I was hoping I could consider the gss-proxy work committed at this point
and pile any fixes on top, but... whatever works for you guys, I guess.

--b.

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
>> 
>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> 
>>> Hi J.,
>>> 
>>> After merging the nfsd tree, today's linux-next build (powerpc
>>> ppc64_defconfig) failed like this:
>>> 
>>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
>>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
>>> 
>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
>>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
>>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
>>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
>>> tree that you did not merge).
>>> 
>>> I don't know how to fix this, so I have used the nfsd tree from
>>> next-20130426 for today.
>> 
>> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
> 
> I'm happy to take those patches whenever you consider them ready.  Would
> that fix the problem?

Someone would need to modify the gssproxy patches to use the new interfaces.

> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
> of *that*?

That doesn't seem right to me.

> I was hoping I could consider the gss-proxy work committed at this point
> and pile any fixes on top, but... whatever works for you guys, I guess.
> 
> --b.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[Trond Myklebust]

On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >> 
> >> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >> 
> >>> Hi J.,
> >>> 
> >>> After merging the nfsd tree, today's linux-next build (powerpc
> >>> ppc64_defconfig) failed like this:
> >>> 
> >>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> >>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> >>> 
> >>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> >>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
> >>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> >>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> >>> tree that you did not merge).
> >>> 
> >>> I don't know how to fix this, so I have used the nfsd tree from
> >>> next-20130426 for today.
> >> 
> >> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
> > 
> > I'm happy to take those patches whenever you consider them ready.  Would
> > that fix the problem?
> 
> Someone would need to modify the gssproxy patches to use the new interfaces.
> 
> > Also: it looks like 030d794bf498 "SUNRPC: Introduce
> > rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> > nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
> > of *that*?
> 
> That doesn't seem right to me.

I've now pulled the rpcsec_gss changes into the nfs-for-next. The main
reason why they were not pulled in earlier was due to uncertainty what
to do about the increase in "AUTH_GSS upcall timed out." syslog
warnings.

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 12:21 PM, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
>> 
>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
>>>> 
>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>> 
>>>>> Hi J.,
>>>>> 
>>>>> After merging the nfsd tree, today's linux-next build (powerpc
>>>>> ppc64_defconfig) failed like this:
>>>>> 
>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
>>>>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
>>>>> 
>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
>>>>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
>>>>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
>>>>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
>>>>> tree that you did not merge).
>>>>> 
>>>>> I don't know how to fix this, so I have used the nfsd tree from
>>>>> next-20130426 for today.
>>>> 
>>>> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
>>> 
>>> I'm happy to take those patches whenever you consider them ready.  Would
>>> that fix the problem?
>> 
>> Someone would need to modify the gssproxy patches to use the new interfaces.
>> 
>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
>>> of *that*?
>> 
>> That doesn't seem right to me.
> 
> I've now pulled the rpcsec_gss changes into the nfs-for-next. The main
> reason why they were not pulled in earlier was due to uncertainty what
> to do about the increase in "AUTH_GSS upcall timed out." syslog
> warnings.

Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and rpcauth_get_pseudoflavor() APIs, which are replacements for direct calls into the GSS mech switch.  These APIs are a little more generic, and more robust in the face of unloaded GSS kernel modules.

Instead of gss_mech_get_by_OID(), I suspect you want rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[Simo Sorce]

On Mon, 2013-04-29 at 13:04 -0400, Chuck Lever wrote:
> On Apr 29, 2013, at 12:21 PM, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> 
> > On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> >> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >> 
> >>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >>>> 
> >>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >>>> 
> >>>>> Hi J.,
> >>>>> 
> >>>>> After merging the nfsd tree, today's linux-next build (powerpc
> >>>>> ppc64_defconfig) failed like this:
> >>>>> 
> >>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> >>>>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> >>>>> 
> >>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> >>>>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
> >>>>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> >>>>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> >>>>> tree that you did not merge).
> >>>>> 
> >>>>> I don't know how to fix this, so I have used the nfsd tree from
> >>>>> next-20130426 for today.
> >>>> 
> >>>> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
> >>> 
> >>> I'm happy to take those patches whenever you consider them ready.  Would
> >>> that fix the problem?
> >> 
> >> Someone would need to modify the gssproxy patches to use the new interfaces.
> >> 
> >>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> >>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> >>> nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
> >>> of *that*?
> >> 
> >> That doesn't seem right to me.
> > 
> > I've now pulled the rpcsec_gss changes into the nfs-for-next. The main
> > reason why they were not pulled in earlier was due to uncertainty what
> > to do about the increase in "AUTH_GSS upcall timed out." syslog
> > warnings.
> 
> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
> calls into the GSS mech switch.  These APIs are a little more generic,
> and more robust in the face of unloaded GSS kernel modules.
> 
> Instead of gss_mech_get_by_OID(), I suspect you want
> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.

The simplest way would be to make it not static again.

In my code we are using gss_mech_get_by_OID() instead of
gss_mech_get_by_name() because we have a OID passed down from gssproxy.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
> 
> On Apr 29, 2013, at 12:21 PM, Trond Myklebust
> <trond.myklebust@fys.uio.no> wrote:
> 
> > On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> >> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields"
> >> <bfields@fieldses.org> wrote:
> >> 
> >>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >>>> 
> >>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell
> >>>> <sfr@canb.auug.org.au> wrote:
> >>>> 
> >>>>> Hi J.,
> >>>>> 
> >>>>> After merging the nfsd tree, today's linux-next build (powerpc
> >>>>> ppc64_defconfig) failed like this:
> >>>>> 
> >>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function
> >>>>> 'gss_proxy_save_rsc': net/sunrpc/auth_gss/svcauth_gss.c:1182:3:
> >>>>> error: implicit declaration of function 'gss_mech_get_by_OID'
> >>>>> [-Werror=implicit-function-declaration]
> >>>>> 
> >>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for
> >>>>> server RPCGSS authentication").  gss_mech_get_by_OID() made
> >>>>> static to net/sunrpc/auth_gss/gss_mech_switch.c by commit
> >>>>> 9568c5e9a61d ("SUNRPC: Introduce rpcauth_get_pseudoflavor()") in
> >>>>> the nfs tree (part of the nfs tree that you did not merge).
> >>>>> 
> >>>>> I don't know how to fix this, so I have used the nfsd tree from
> >>>>> next-20130426 for today.
> >>>> 
> >>>> Bruce, it might make sense for me to submit the three server-side
> >>>> RPC GSS patches, and then you can rebase the gssproxy work on top
> >>>> of those.  Let me know how you would like to proceed.
> >>> 
> >>> I'm happy to take those patches whenever you consider them ready.
> >>> Would that fix the problem?
> >> 
> >> Someone would need to modify the gssproxy patches to use the new
> >> interfaces.
> >> 
> >>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> >>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> >>> nfs-for-next.  I'm not sure what that means--is it safe to rebase
> >>> on top of *that*?
> >> 
> >> That doesn't seem right to me.
> > 
> > I've now pulled the rpcsec_gss changes into the nfs-for-next. The
> > main reason why they were not pulled in earlier was due to
> > uncertainty what to do about the increase in "AUTH_GSS upcall timed
> > out." syslog warnings.
> 
> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
> calls into the GSS mech switch.  These APIs are a little more generic,
> and more robust in the face of unloaded GSS kernel modules.
> 
> Instead of gss_mech_get_by_OID(), I suspect you want
> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.

It's doing

		status = -EOPNOTSUPP;
		gm = gss_mech_get_by_OID(&ud->mech_oid);
		if (!gm)
			goto out;
		status = -EINVAL;
		status = gss_import_sec_context(ud->out_handle.data,
						ud->out_handle.len,
						gm, &rsci.mechctx,
						&expiry, GFP_KERNEL);
		if (status)
			goto out;

So we need a way to get from an OID and some mechanism-specific data to
a context.

Looks to me like we just want to re-export gss_mech_get_by_OID().

--b.

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
>> 
>> On Apr 29, 2013, at 12:21 PM, Trond Myklebust
>> <trond.myklebust@fys.uio.no> wrote:
>> 
>>> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
>>>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields"
>>>> <bfields@fieldses.org> wrote:
>>>> 
>>>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
>>>>>> 
>>>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell
>>>>>> <sfr@canb.auug.org.au> wrote:
>>>>>> 
>>>>>>> Hi J.,
>>>>>>> 
>>>>>>> After merging the nfsd tree, today's linux-next build (powerpc
>>>>>>> ppc64_defconfig) failed like this:
>>>>>>> 
>>>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function
>>>>>>> 'gss_proxy_save_rsc': net/sunrpc/auth_gss/svcauth_gss.c:1182:3:
>>>>>>> error: implicit declaration of function 'gss_mech_get_by_OID'
>>>>>>> [-Werror=implicit-function-declaration]
>>>>>>> 
>>>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for
>>>>>>> server RPCGSS authentication").  gss_mech_get_by_OID() made
>>>>>>> static to net/sunrpc/auth_gss/gss_mech_switch.c by commit
>>>>>>> 9568c5e9a61d ("SUNRPC: Introduce rpcauth_get_pseudoflavor()") in
>>>>>>> the nfs tree (part of the nfs tree that you did not merge).
>>>>>>> 
>>>>>>> I don't know how to fix this, so I have used the nfsd tree from
>>>>>>> next-20130426 for today.
>>>>>> 
>>>>>> Bruce, it might make sense for me to submit the three server-side
>>>>>> RPC GSS patches, and then you can rebase the gssproxy work on top
>>>>>> of those.  Let me know how you would like to proceed.
>>>>> 
>>>>> I'm happy to take those patches whenever you consider them ready.
>>>>> Would that fix the problem?
>>>> 
>>>> Someone would need to modify the gssproxy patches to use the new
>>>> interfaces.
>>>> 
>>>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
>>>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
>>>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase
>>>>> on top of *that*?
>>>> 
>>>> That doesn't seem right to me.
>>> 
>>> I've now pulled the rpcsec_gss changes into the nfs-for-next. The
>>> main reason why they were not pulled in earlier was due to
>>> uncertainty what to do about the increase in "AUTH_GSS upcall timed
>>> out." syslog warnings.
>> 
>> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
>> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
>> calls into the GSS mech switch.  These APIs are a little more generic,
>> and more robust in the face of unloaded GSS kernel modules.
>> 
>> Instead of gss_mech_get_by_OID(), I suspect you want
>> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.
> 
> It's doing
> 
> 		status = -EOPNOTSUPP;
> 		gm = gss_mech_get_by_OID(&ud->mech_oid);
> 		if (!gm)
> 			goto out;
> 		status = -EINVAL;
> 		status = gss_import_sec_context(ud->out_handle.data,
> 						ud->out_handle.len,
> 						gm, &rsci.mechctx,
> 						&expiry, GFP_KERNEL);
> 		if (status)
> 			goto out;
> 
> So we need a way to get from an OID and some mechanism-specific data to
> a context.
> 
> Looks to me like we just want to re-export gss_mech_get_by_OID().

The reason for the new wrappers is to load the kernel modules properly before trying to discover the OID -> mechanism mapping.

Where are you calling it from?  If it's from outside of the GSS module, how do you guarantee the rpc_gss_auth module is loaded?  What if the GSS mechanism for that OID isn't loaded?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[Simo Sorce]

On Mon, 2013-04-29 at 13:47 -0400, Chuck Lever wrote:
> On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
> >> 
> >> On Apr 29, 2013, at 12:21 PM, Trond Myklebust
> >> <trond.myklebust@fys.uio.no> wrote:
> >> 
> >>> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> >>>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields"
> >>>> <bfields@fieldses.org> wrote:
> >>>> 
> >>>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >>>>>> 
> >>>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell
> >>>>>> <sfr@canb.auug.org.au> wrote:
> >>>>>> 
> >>>>>>> Hi J.,
> >>>>>>> 
> >>>>>>> After merging the nfsd tree, today's linux-next build (powerpc
> >>>>>>> ppc64_defconfig) failed like this:
> >>>>>>> 
> >>>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function
> >>>>>>> 'gss_proxy_save_rsc': net/sunrpc/auth_gss/svcauth_gss.c:1182:3:
> >>>>>>> error: implicit declaration of function 'gss_mech_get_by_OID'
> >>>>>>> [-Werror=implicit-function-declaration]
> >>>>>>> 
> >>>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for
> >>>>>>> server RPCGSS authentication").  gss_mech_get_by_OID() made
> >>>>>>> static to net/sunrpc/auth_gss/gss_mech_switch.c by commit
> >>>>>>> 9568c5e9a61d ("SUNRPC: Introduce rpcauth_get_pseudoflavor()") in
> >>>>>>> the nfs tree (part of the nfs tree that you did not merge).
> >>>>>>> 
> >>>>>>> I don't know how to fix this, so I have used the nfsd tree from
> >>>>>>> next-20130426 for today.
> >>>>>> 
> >>>>>> Bruce, it might make sense for me to submit the three server-side
> >>>>>> RPC GSS patches, and then you can rebase the gssproxy work on top
> >>>>>> of those.  Let me know how you would like to proceed.
> >>>>> 
> >>>>> I'm happy to take those patches whenever you consider them ready.
> >>>>> Would that fix the problem?
> >>>> 
> >>>> Someone would need to modify the gssproxy patches to use the new
> >>>> interfaces.
> >>>> 
> >>>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> >>>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> >>>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase
> >>>>> on top of *that*?
> >>>> 
> >>>> That doesn't seem right to me.
> >>> 
> >>> I've now pulled the rpcsec_gss changes into the nfs-for-next. The
> >>> main reason why they were not pulled in earlier was due to
> >>> uncertainty what to do about the increase in "AUTH_GSS upcall timed
> >>> out." syslog warnings.
> >> 
> >> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
> >> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
> >> calls into the GSS mech switch.  These APIs are a little more generic,
> >> and more robust in the face of unloaded GSS kernel modules.
> >> 
> >> Instead of gss_mech_get_by_OID(), I suspect you want
> >> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.
> > 
> > It's doing
> > 
> > 		status = -EOPNOTSUPP;
> > 		gm = gss_mech_get_by_OID(&ud->mech_oid);
> > 		if (!gm)
> > 			goto out;
> > 		status = -EINVAL;
> > 		status = gss_import_sec_context(ud->out_handle.data,
> > 						ud->out_handle.len,
> > 						gm, &rsci.mechctx,
> > 						&expiry, GFP_KERNEL);
> > 		if (status)
> > 			goto out;
> > 
> > So we need a way to get from an OID and some mechanism-specific data to
> > a context.
> > 
> > Looks to me like we just want to re-export gss_mech_get_by_OID().
> 
> The reason for the new wrappers is to load the kernel modules properly before trying to discover the OID -> mechanism mapping.
> 
> Where are you calling it from?  If it's from outside of the GSS module, how do you guarantee the rpc_gss_auth module is loaded?  What if the GSS mechanism for that OID isn't loaded?
> 

Does gss_mech_get_by_name() do the loading ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Mon, Apr 29, 2013 at 01:47:16PM -0400, Chuck Lever wrote:
> 
> On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
> >> 
> >> On Apr 29, 2013, at 12:21 PM, Trond Myklebust
> >> <trond.myklebust@fys.uio.no> wrote:
> >> 
> >>> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> >>>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields"
> >>>> <bfields@fieldses.org> wrote:
> >>>> 
> >>>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >>>>>> 
> >>>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell
> >>>>>> <sfr@canb.auug.org.au> wrote:
> >>>>>> 
> >>>>>>> Hi J.,
> >>>>>>> 
> >>>>>>> After merging the nfsd tree, today's linux-next build (powerpc
> >>>>>>> ppc64_defconfig) failed like this:
> >>>>>>> 
> >>>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function
> >>>>>>> 'gss_proxy_save_rsc': net/sunrpc/auth_gss/svcauth_gss.c:1182:3:
> >>>>>>> error: implicit declaration of function 'gss_mech_get_by_OID'
> >>>>>>> [-Werror=implicit-function-declaration]
> >>>>>>> 
> >>>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for
> >>>>>>> server RPCGSS authentication").  gss_mech_get_by_OID() made
> >>>>>>> static to net/sunrpc/auth_gss/gss_mech_switch.c by commit
> >>>>>>> 9568c5e9a61d ("SUNRPC: Introduce rpcauth_get_pseudoflavor()") in
> >>>>>>> the nfs tree (part of the nfs tree that you did not merge).
> >>>>>>> 
> >>>>>>> I don't know how to fix this, so I have used the nfsd tree from
> >>>>>>> next-20130426 for today.
> >>>>>> 
> >>>>>> Bruce, it might make sense for me to submit the three server-side
> >>>>>> RPC GSS patches, and then you can rebase the gssproxy work on top
> >>>>>> of those.  Let me know how you would like to proceed.
> >>>>> 
> >>>>> I'm happy to take those patches whenever you consider them ready.
> >>>>> Would that fix the problem?
> >>>> 
> >>>> Someone would need to modify the gssproxy patches to use the new
> >>>> interfaces.
> >>>> 
> >>>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> >>>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> >>>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase
> >>>>> on top of *that*?
> >>>> 
> >>>> That doesn't seem right to me.
> >>> 
> >>> I've now pulled the rpcsec_gss changes into the nfs-for-next. The
> >>> main reason why they were not pulled in earlier was due to
> >>> uncertainty what to do about the increase in "AUTH_GSS upcall timed
> >>> out." syslog warnings.
> >> 
> >> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
> >> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
> >> calls into the GSS mech switch.  These APIs are a little more generic,
> >> and more robust in the face of unloaded GSS kernel modules.
> >> 
> >> Instead of gss_mech_get_by_OID(), I suspect you want
> >> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.
> > 
> > It's doing
> > 
> > 		status = -EOPNOTSUPP;
> > 		gm = gss_mech_get_by_OID(&ud->mech_oid);
> > 		if (!gm)
> > 			goto out;
> > 		status = -EINVAL;
> > 		status = gss_import_sec_context(ud->out_handle.data,
> > 						ud->out_handle.len,
> > 						gm, &rsci.mechctx,
> > 						&expiry, GFP_KERNEL);
> > 		if (status)
> > 			goto out;
> > 
> > So we need a way to get from an OID and some mechanism-specific data to
> > a context.
> > 
> > Looks to me like we just want to re-export gss_mech_get_by_OID().
> 
> The reason for the new wrappers is to load the kernel modules properly before trying to discover the OID -> mechanism mapping.
> 
> Where are you calling it from?  If it's from outside of the GSS module, how do you guarantee the rpc_gss_auth module is loaded?  What if the GSS mechanism for that OID isn't loaded?

Sorry, I should have said just "remove static from", not
"re-export"--it's in the same module.  So there should be no problem
here, right?

--b.

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 1:59 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Apr 29, 2013 at 01:47:16PM -0400, Chuck Lever wrote:
>> 
>> On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
>> 
>>> On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
>>>> 
>>>> On Apr 29, 2013, at 12:21 PM, Trond Myklebust
>>>> <trond.myklebust@fys.uio.no> wrote:
>>>> 
>>>>> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
>>>>>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields"
>>>>>> <bfields@fieldses.org> wrote:
>>>>>> 
>>>>>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
>>>>>>>> 
>>>>>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell
>>>>>>>> <sfr@canb.auug.org.au> wrote:
>>>>>>>> 
>>>>>>>>> Hi J.,
>>>>>>>>> 
>>>>>>>>> After merging the nfsd tree, today's linux-next build (powerpc
>>>>>>>>> ppc64_defconfig) failed like this:
>>>>>>>>> 
>>>>>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function
>>>>>>>>> 'gss_proxy_save_rsc': net/sunrpc/auth_gss/svcauth_gss.c:1182:3:
>>>>>>>>> error: implicit declaration of function 'gss_mech_get_by_OID'
>>>>>>>>> [-Werror=implicit-function-declaration]
>>>>>>>>> 
>>>>>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for
>>>>>>>>> server RPCGSS authentication").  gss_mech_get_by_OID() made
>>>>>>>>> static to net/sunrpc/auth_gss/gss_mech_switch.c by commit
>>>>>>>>> 9568c5e9a61d ("SUNRPC: Introduce rpcauth_get_pseudoflavor()") in
>>>>>>>>> the nfs tree (part of the nfs tree that you did not merge).
>>>>>>>>> 
>>>>>>>>> I don't know how to fix this, so I have used the nfsd tree from
>>>>>>>>> next-20130426 for today.
>>>>>>>> 
>>>>>>>> Bruce, it might make sense for me to submit the three server-side
>>>>>>>> RPC GSS patches, and then you can rebase the gssproxy work on top
>>>>>>>> of those.  Let me know how you would like to proceed.
>>>>>>> 
>>>>>>> I'm happy to take those patches whenever you consider them ready.
>>>>>>> Would that fix the problem?
>>>>>> 
>>>>>> Someone would need to modify the gssproxy patches to use the new
>>>>>> interfaces.
>>>>>> 
>>>>>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
>>>>>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
>>>>>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase
>>>>>>> on top of *that*?
>>>>>> 
>>>>>> That doesn't seem right to me.
>>>>> 
>>>>> I've now pulled the rpcsec_gss changes into the nfs-for-next. The
>>>>> main reason why they were not pulled in earlier was due to
>>>>> uncertainty what to do about the increase in "AUTH_GSS upcall timed
>>>>> out." syslog warnings.
>>>> 
>>>> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
>>>> rpcauth_get_pseudoflavor() APIs, which are replacements for direct
>>>> calls into the GSS mech switch.  These APIs are a little more generic,
>>>> and more robust in the face of unloaded GSS kernel modules.
>>>> 
>>>> Instead of gss_mech_get_by_OID(), I suspect you want
>>>> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy code.
>>> 
>>> It's doing
>>> 
>>> 		status = -EOPNOTSUPP;
>>> 		gm = gss_mech_get_by_OID(&ud->mech_oid);
>>> 		if (!gm)
>>> 			goto out;
>>> 		status = -EINVAL;
>>> 		status = gss_import_sec_context(ud->out_handle.data,
>>> 						ud->out_handle.len,
>>> 						gm, &rsci.mechctx,
>>> 						&expiry, GFP_KERNEL);
>>> 		if (status)
>>> 			goto out;
>>> 
>>> So we need a way to get from an OID and some mechanism-specific data to
>>> a context.
>>> 
>>> Looks to me like we just want to re-export gss_mech_get_by_OID().
>> 
>> The reason for the new wrappers is to load the kernel modules properly before trying to discover the OID -> mechanism mapping.
>> 
>> Where are you calling it from?  If it's from outside of the GSS module, how do you guarantee the rpc_gss_auth module is loaded?  What if the GSS mechanism for that OID isn't loaded?
> 
> Sorry, I should have said just "remove static from", not
> "re-export"--it's in the same module.  So there should be no problem
> here, right?

OK, gotcha.  Architecturally outside of the mech switch I'd like to see OIDs passed around embedded in GSS tuples rather than by themselves.

An alternative would be to use gss_mech_get_by_name(), which is already visible, loads GSS mechanism modules automatically, and returns struct gss_api_mech *.  For NFS, we should already have a clean mapping of mechanism name to pseudoflavor to GSS tuple.  Looks like rsc_parse() already uses this API.

Do you have gssproxy patches published in a git tree somewhere I could review?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Mon, Apr 29, 2013 at 02:30:33PM -0400, Chuck Lever wrote:
> 
> On Apr 29, 2013, at 1:59 PM, "J. Bruce Fields" <bfields@fieldses.org>
> wrote:
> 
> > On Mon, Apr 29, 2013 at 01:47:16PM -0400, Chuck Lever wrote:
> >> 
> >> On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields"
> >> <bfields@fieldses.org> wrote:
> >> 
> >>> On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
> >>>> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
> >>>> rpcauth_get_pseudoflavor() APIs, which are replacements for
> >>>> direct calls into the GSS mech switch.  These APIs are a little
> >>>> more generic, and more robust in the face of unloaded GSS kernel
> >>>> modules.
> >>>> 
> >>>> Instead of gss_mech_get_by_OID(), I suspect you want
> >>>> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy
> >>>> code.
> >>> 
> >>> It's doing
> >>> 
> >>> 		status = -EOPNOTSUPP; gm =
> >>> 		gss_mech_get_by_OID(&ud->mech_oid); if (!gm) goto out;
> >>> 		status = -EINVAL; status =
> >>> 		gss_import_sec_context(ud->out_handle.data,
> >>> 		ud->out_handle.len, gm, &rsci.mechctx, &expiry,
> >>> 		GFP_KERNEL); if (status) goto out;
> >>> 
> >>> So we need a way to get from an OID and some mechanism-specific
> >>> data to a context.
> >>> 
> >>> Looks to me like we just want to re-export gss_mech_get_by_OID().
> >> 
> >> The reason for the new wrappers is to load the kernel modules
> >> properly before trying to discover the OID -> mechanism mapping.
> >> 
> >> Where are you calling it from?  If it's from outside of the GSS
> >> module, how do you guarantee the rpc_gss_auth module is loaded?
> >> What if the GSS mechanism for that OID isn't loaded?
> > 
> > Sorry, I should have said just "remove static from", not
> > "re-export"--it's in the same module.  So there should be no problem
> > here, right?
> 
> OK, gotcha.  Architecturally outside of the mech switch I'd like to
> see OIDs passed around embedded in GSS tuples rather than by
> themselves.

I'm not sure what you mean.  When I accept a gss context I don't yet
know what service or qop it's going to be used with, I only know the
mechanism OID.

> An alternative would be to use gss_mech_get_by_name(), which is
> already visible, loads GSS mechanism modules automatically, and
> returns struct gss_api_mech *.  For NFS, we should already have a
> clean mapping of mechanism name to pseudoflavor to GSS tuple.  Looks
> like rsc_parse() already uses this API.

We don't have a name here, only an OID.

> Do you have gssproxy patches published in a git tree somewhere I could
> review?

It's in my for-3.10 branch.

Which is more or less what I plan to submit for 3.10, so I'd prefer not
to rebase it at this point.

It looks like just removing "static" would resolve the immediate
conflict, is that right?  And then I'd happily help deal with cleaning
up the API as followup work.

--b.

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 2:57 PM, J. Bruce Fields <bfields@fieldses.org> wrote:

> On Mon, Apr 29, 2013 at 02:30:33PM -0400, Chuck Lever wrote:
>> 
>> On Apr 29, 2013, at 1:59 PM, "J. Bruce Fields" <bfields@fieldses.org>
>> wrote:
>> 
>>> On Mon, Apr 29, 2013 at 01:47:16PM -0400, Chuck Lever wrote:
>>>> 
>>>> On Apr 29, 2013, at 1:38 PM, "J. Bruce Fields"
>>>> <bfields@fieldses.org> wrote:
>>>> 
>>>>> On Mon, Apr 29, 2013 at 01:04:01PM -0400, Chuck Lever wrote:
>>>>>> Trond's nfs-for-next now has the new rpcauth_get_gssinfo() and
>>>>>> rpcauth_get_pseudoflavor() APIs, which are replacements for
>>>>>> direct calls into the GSS mech switch.  These APIs are a little
>>>>>> more generic, and more robust in the face of unloaded GSS kernel
>>>>>> modules.
>>>>>> 
>>>>>> Instead of gss_mech_get_by_OID(), I suspect you want
>>>>>> rpcauth_get_pseudoflavor(), but I haven't looked at the gssproxy
>>>>>> code.
>>>>> 
>>>>> It's doing
>>>>> 
>>>>> 		status = -EOPNOTSUPP; gm =
>>>>> 		gss_mech_get_by_OID(&ud->mech_oid); if (!gm) goto out;
>>>>> 		status = -EINVAL; status =
>>>>> 		gss_import_sec_context(ud->out_handle.data,
>>>>> 		ud->out_handle.len, gm, &rsci.mechctx, &expiry,
>>>>> 		GFP_KERNEL); if (status) goto out;
>>>>> 
>>>>> So we need a way to get from an OID and some mechanism-specific
>>>>> data to a context.
>>>>> 
>>>>> Looks to me like we just want to re-export gss_mech_get_by_OID().
>>>> 
>>>> The reason for the new wrappers is to load the kernel modules
>>>> properly before trying to discover the OID -> mechanism mapping.
>>>> 
>>>> Where are you calling it from?  If it's from outside of the GSS
>>>> module, how do you guarantee the rpc_gss_auth module is loaded?
>>>> What if the GSS mechanism for that OID isn't loaded?
>>> 
>>> Sorry, I should have said just "remove static from", not
>>> "re-export"--it's in the same module.  So there should be no problem
>>> here, right?
>> 
>> OK, gotcha.  Architecturally outside of the mech switch I'd like to
>> see OIDs passed around embedded in GSS tuples rather than by
>> themselves.
> 
> I'm not sure what you mean.  When I accept a gss context I don't yet
> know what service or qop it's going to be used with, I only know the
> mechanism OID.

RPC server GSS support didn't need the gss_mech_get_by_OID() interface before gssproxy, so I'm trying to understand why it is needed now.

But it sounds like you do need it now, so go ahead and make gss_mech_get_by_OID() global within the AUTH_GSS module. 

> 
>> An alternative would be to use gss_mech_get_by_name(), which is
>> already visible, loads GSS mechanism modules automatically, and
>> returns struct gss_api_mech *.  For NFS, we should already have a
>> clean mapping of mechanism name to pseudoflavor to GSS tuple.  Looks
>> like rsc_parse() already uses this API.
> 
> We don't have a name here, only an OID.
> 
>> Do you have gssproxy patches published in a git tree somewhere I could
>> review?
> 
> It's in my for-3.10 branch.
> 
> Which is more or less what I plan to submit for 3.10, so I'd prefer not
> to rebase it at this point.
> 
> It looks like just removing "static" would resolve the immediate
> conflict, is that right?  And then I'd happily help deal with cleaning
> up the API as followup work.



-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[Simo Sorce]

On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >> 
> >> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >> 
> >>> Hi J.,
> >>> 
> >>> After merging the nfsd tree, today's linux-next build (powerpc
> >>> ppc64_defconfig) failed like this:
> >>> 
> >>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> >>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> >>> 
> >>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> >>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
> >>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> >>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> >>> tree that you did not merge).
> >>> 
> >>> I don't know how to fix this, so I have used the nfsd tree from
> >>> next-20130426 for today.
> >> 
> >> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
> > 
> > I'm happy to take those patches whenever you consider them ready.  Would
> > that fix the problem?
> 
> Someone would need to modify the gssproxy patches to use the new interfaces.
> 
> > Also: it looks like 030d794bf498 "SUNRPC: Introduce
> > rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> > nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
> > of *that*?
> 
> That doesn't seem right to me.

GSS-Proxy patches are 1 year old and we've been delayed once already to
accomodate the containers work, maybe it's time for your patches to be
rebased on gssproxy ones ? :-)

Simo. 

-- 
Simo Sorce * Red Hat, Inc * New York

Re: linux-next: build failure after merge of the nfsd tree

[Chuck Lever]

On Apr 29, 2013, at 12:29 PM, Simo Sorce <simo@redhat.com> wrote:

> On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
>> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
>> 
>>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
>>>> 
>>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>> 
>>>>> Hi J.,
>>>>> 
>>>>> After merging the nfsd tree, today's linux-next build (powerpc
>>>>> ppc64_defconfig) failed like this:
>>>>> 
>>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
>>>>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
>>>>> 
>>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
>>>>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
>>>>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
>>>>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
>>>>> tree that you did not merge).
>>>>> 
>>>>> I don't know how to fix this, so I have used the nfsd tree from
>>>>> next-20130426 for today.
>>>> 
>>>> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
>>> 
>>> I'm happy to take those patches whenever you consider them ready.  Would
>>> that fix the problem?
>> 
>> Someone would need to modify the gssproxy patches to use the new interfaces.
>> 
>>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
>>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
>>> nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
>>> of *that*?
>> 
>> That doesn't seem right to me.
> 
> GSS-Proxy patches are 1 year old and we've been delayed once already to
> accomodate the containers work, maybe it's time for your patches to be
> rebased on gssproxy ones ? :-)

Don't sweat it.  IMO this is a simple merge problem, unlike the containers work.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: linux-next: build failure after merge of the nfsd tree

[Simo Sorce]

On Mon, 2013-04-29 at 12:37 -0400, Chuck Lever wrote:
> On Apr 29, 2013, at 12:29 PM, Simo Sorce <simo@redhat.com> wrote:
> 
> > On Mon, 2013-04-29 at 12:05 -0400, Chuck Lever wrote:
> >> On Apr 29, 2013, at 11:45 AM, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >> 
> >>> On Mon, Apr 29, 2013 at 10:53:37AM -0400, Chuck Lever wrote:
> >>>> 
> >>>> On Apr 28, 2013, at 9:24 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >>>> 
> >>>>> Hi J.,
> >>>>> 
> >>>>> After merging the nfsd tree, today's linux-next build (powerpc
> >>>>> ppc64_defconfig) failed like this:
> >>>>> 
> >>>>> net/sunrpc/auth_gss/svcauth_gss.c: In function 'gss_proxy_save_rsc':
> >>>>> net/sunrpc/auth_gss/svcauth_gss.c:1182:3: error: implicit declaration of function 'gss_mech_get_by_OID' [-Werror=implicit-function-declaration]
> >>>>> 
> >>>>> Caused byc ommit 030d794bf498 ("SUNRPC: Use gssproxy upcall for server
> >>>>> RPCGSS authentication").  gss_mech_get_by_OID() made static to
> >>>>> net/sunrpc/auth_gss/gss_mech_switch.c by commit 9568c5e9a61d ("SUNRPC:
> >>>>> Introduce rpcauth_get_pseudoflavor()") in the nfs tree (part of the nfs
> >>>>> tree that you did not merge).
> >>>>> 
> >>>>> I don't know how to fix this, so I have used the nfsd tree from
> >>>>> next-20130426 for today.
> >>>> 
> >>>> Bruce, it might make sense for me to submit the three server-side RPC GSS patches, and then you can rebase the gssproxy work on top of those.  Let me know how you would like to proceed.
> >>> 
> >>> I'm happy to take those patches whenever you consider them ready.  Would
> >>> that fix the problem?
> >> 
> >> Someone would need to modify the gssproxy patches to use the new interfaces.
> >> 
> >>> Also: it looks like 030d794bf498 "SUNRPC: Introduce
> >>> rpcauth_get_pseudoflavor()" is in Trond's linux-next, but not his
> >>> nfs-for-next.  I'm not sure what that means--is it safe to rebase on top
> >>> of *that*?
> >> 
> >> That doesn't seem right to me.
> > 
> > GSS-Proxy patches are 1 year old and we've been delayed once already to
> > accomodate the containers work, maybe it's time for your patches to be
> > rebased on gssproxy ones ? :-)
> 
> Don't sweat it.  IMO this is a simple merge problem, unlike the containers work.

Glad to hear that.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

Hi all,

After merging the nfsd tree, today's linux-next build (x86_64
allmodconfig) failed like this:

kernel/trace/trace_hwlat.c:329:12: error: conflicting types for 'kthread_fn'
  329 | static int kthread_fn(void *data)
      |            ^~~~~~~~~~
In file included from kernel/trace/trace_hwlat.c:40:
include/linux/kthread.h:60:7: note: previous declaration of 'kthread_fn' was here
   60 | void *kthread_fn(struct task_struct *k);
      |       ^~~~~~~~~~

Caused by commit

  7df082e85764 ("kthread: save thread function")

I have used the nfsd tree from next-20200507 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Fri, May 08, 2020 at 10:47:20AM +1000, Stephen Rothwell wrote:
> After merging the nfsd tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> kernel/trace/trace_hwlat.c:329:12: error: conflicting types for 'kthread_fn'
>   329 | static int kthread_fn(void *data)
>       |            ^~~~~~~~~~
> In file included from kernel/trace/trace_hwlat.c:40:
> include/linux/kthread.h:60:7: note: previous declaration of 'kthread_fn' was here
>    60 | void *kthread_fn(struct task_struct *k);
>       |       ^~~~~~~~~~
> 
> Caused by commit
> 
>   7df082e85764 ("kthread: save thread function")
> 
> I have used the nfsd tree from next-20200507 for today.

Whoops, I forgot to say thanks for this report.  I renamed the
kthread_fn in my patches to kthread_func.

--b.

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

Hi all,

After merging the nfsd tree, today's linux-next build (x86_64
allmodconfig) failed like this:

fs/nfs/dns_resolve.c: In function 'nfs_dns_ent_put':
fs/nfs/dns_resolve.c:119:11: error: passing argument 1 of 'call_rcu_sched' from incompatible pointer type [-Werror=incompatible-pointer-types]
  call_rcu(item, nfs_dns_ent_free_rcu);
           ^~~~
In file included from include/linux/srcu.h:33,
                 from include/linux/notifier.h:16,
                 from include/linux/memory_hotplug.h:7,
                 from include/linux/mmzone.h:758,
                 from include/linux/gfp.h:6,
                 from include/linux/umh.h:4,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from fs/nfs/dns_resolve.c:36:
include/linux/rcupdate.h:59:38: note: expected 'struct callback_head *' but argument is of type 'struct nfs_dns_ent *'
 void call_rcu_sched(struct rcu_head *head, rcu_callback_t func);
                     ~~~~~~~~~~~~~~~~~^~~~

Caused by commit

  fd497f1e40d9 ("NFS: Lockless DNS lookups")

I have used the nfsd tree from next-20181004 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2252 bytes --]

Hi all,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'svc_rdma_handle_bc_reply':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:67:21: error: 'struct rpc_xprt' has no member named 'recv_lock'; did you mean 'reserve_lock'?
  spin_unlock(&xprt->recv_lock);
                     ^~~~~~~~~
                     reserve_lock
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:79:19: error: 'struct rpc_xprt' has no member named 'recv_lock'; did you mean 'reserve_lock'?
  spin_lock(&xprt->recv_lock);
                   ^~~~~~~~~
                   reserve_lock

Caused by commit

  cf4f6fd48dac ("svcrdma: Remove ->release_rqst call in bc reply handler")

interacting with commit

  75c84151a9dc ("SUNRPC: Rename xprt->recv_lock to xprt->queue_lock")

from the nfs tree.

I have added the following merge fix patch for today:

From b249a74ca8c4f992e2530f95218ea4f2ed72e0b9 Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 5 Oct 2018 09:51:30 +1000
Subject: [PATCH] svcrdma: fix up for recv_lock rename

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
index 19e34a6aa583..2d176a48543a 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
@@ -64,7 +64,7 @@ int svc_rdma_handle_bc_reply(struct rpc_xprt *xprt, __be32 *rdma_resp,
 		goto out_unlock;
 	memcpy(dst->iov_base, p, len);
 	xprt_pin_rqst(req);
-	spin_unlock(&xprt->recv_lock);
+	spin_unlock(&xprt->queue_lock);
 
 	credits = be32_to_cpup(rdma_resp + 2);
 	if (credits == 0)
@@ -76,7 +76,7 @@ int svc_rdma_handle_bc_reply(struct rpc_xprt *xprt, __be32 *rdma_resp,
 	xprt->cwnd = credits << RPC_CWNDSHIFT;
 	spin_unlock_bh(&xprt->transport_lock);
 
-	spin_lock(&xprt->recv_lock);
+	spin_lock(&xprt->queue_lock);
 	ret = 0;
 	xprt_complete_rqst(req->rq_task, rcvbuf->len);
 	xprt_unpin_rqst(req);
-- 
2.18.0

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

Hi all,

After merging the nfsd tree, today's linux-next build (x86_64
allmodconfig) failed like this:

fs/nfsd/nfs4proc.c: In function 'nfsd4_getdeviceinfo_release':
fs/nfsd/nfs4proc.c:1356:8: error: 'gdev' undeclared (first use in this function)
  kfree(gdev->gd_device);
        ^
fs/nfsd/nfs4proc.c:1356:8: note: each undeclared identifier is reported only once for each function it appears in
fs/nfsd/nfs4proc.c: At top level:
fs/nfsd/nfs4proc.c:1439:1: warning: return type defaults to 'int' [-Wreturn-type]
 nfsd4_layoutget_release(union nfsd4_op_u *u)
 ^
fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
fs/nfsd/nfs4proc.c:1441:8: error: 'lgp' undeclared (first use in this function)
  kfree(lgp->lg_content);
        ^
fs/nfsd/nfs4proc.c: At top level:
fs/nfsd/nfs4proc.c:2405:17: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
   .op_release = nfsd4_layoutget_release,
                 ^
fs/nfsd/nfs4proc.c:2405:17: note: (near initialization for 'nfsd4_ops[50].op_release')
fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
fs/nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

Caused by commit

  93e92e5d0016 ("nfsd4: define ->op_release for compound ops")

I have used the nfsd tree from next-20170824 for today.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

Hi all,

On Fri, 25 Aug 2017 10:24:20 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the nfsd tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> fs/nfsd/nfs4proc.c: In function 'nfsd4_getdeviceinfo_release':
> fs/nfsd/nfs4proc.c:1356:8: error: 'gdev' undeclared (first use in this function)
>   kfree(gdev->gd_device);
>         ^
> fs/nfsd/nfs4proc.c:1356:8: note: each undeclared identifier is reported only once for each function it appears in
> fs/nfsd/nfs4proc.c: At top level:
> fs/nfsd/nfs4proc.c:1439:1: warning: return type defaults to 'int' [-Wreturn-type]
>  nfsd4_layoutget_release(union nfsd4_op_u *u)
>  ^
> fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
> fs/nfsd/nfs4proc.c:1441:8: error: 'lgp' undeclared (first use in this function)
>   kfree(lgp->lg_content);
>         ^
> fs/nfsd/nfs4proc.c: At top level:
> fs/nfsd/nfs4proc.c:2405:17: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>    .op_release = nfsd4_layoutget_release,
>                  ^
> fs/nfsd/nfs4proc.c:2405:17: note: (near initialization for 'nfsd4_ops[50].op_release')
> fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
> fs/nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
>  }
>  ^
> 
> Caused by commit
> 
>   93e92e5d0016 ("nfsd4: define ->op_release for compound ops")
> 
> I have used the nfsd tree from next-20170824 for today.

I am still getting this failure.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Thu, Aug 31, 2017 at 09:48:14AM +1000, Stephen Rothwell wrote:
> On Fri, 25 Aug 2017 10:24:20 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
...
> > fs/nfsd/nfs4proc.c:2405:17: note: (near initialization for 'nfsd4_ops[50].op_release')
> > fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
> > fs/nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
> >  }
> >  ^
> > 
> > Caused by commit
> > 
> >   93e92e5d0016 ("nfsd4: define ->op_release for compound ops")
> > 
> > I have used the nfsd tree from next-20170824 for today.
> 
> I am still getting this failure.

Apologies, it looks like I'd fixed this locally but not pushed it to the
nfsd-next branch.

--b.

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

Hi,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

fs/nfsd/nfs4state.c: In function 'client_has_state':
fs/nfsd/nfs4state.c:2249:22: error: 'struct nfs4_client' has no member named 'cl_lo_states'
   || !list_empty(&clp->cl_lo_states)
                      ^
fs/nfsd/nfs4state.c:2252:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

Caused by commit

  395b297af7bf ("nfsd: Add layouts checking in client_has_state()")

cl_lo_states is only defined when CONFIG_NFSD_PNFS is set.

I have used the nsfd tree from next-20150720 for today.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Re: linux-next: build failure after merge of the nfsd tree

[Kinglong Mee]

Sorry for my fault.
Please drop the old version and use the new version 3.

thanks,
Kinglong Mee

On 7/21/2015 07:57, Stephen Rothwell wrote:
> Hi,
> 
> After merging the nfsd tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> fs/nfsd/nfs4state.c: In function 'client_has_state':
> fs/nfsd/nfs4state.c:2249:22: error: 'struct nfs4_client' has no member named 'cl_lo_states'
>    || !list_empty(&clp->cl_lo_states)
>                       ^
> fs/nfsd/nfs4state.c:2252:1: warning: control reaches end of non-void function [-Wreturn-type]
>  }
>  ^
> 
> Caused by commit
> 
>   395b297af7bf ("nfsd: Add layouts checking in client_has_state()")
> 
> cl_lo_states is only defined when CONFIG_NFSD_PNFS is set.
> 
> I have used the nsfd tree from next-20150720 for today.
> 

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

Hi,

After merging the nfsd tree, today's linux-next build (powerpc ppc64_defconfig)
failed like this:

net/sunrpc/svc_xprt.c: In function 'svc_recv':
net/sunrpc/svc_xprt.c:776:20: error: 'current_task' undeclared (first use in this function)
   rqstp->rq_task = current_task;
                    ^

Caused by commit 983c684466e0 ("SUNRPC: get rid of the request wait
queue").

I have used the nfsd tree from next-20140818 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

Hi Bruce,

After merging the nfsd tree, today's linux-next build (x86_64 allmodconfig)
failed like this:


fs/nfsd/nfs4xdr.c: In function 'nfsd4_encode_security_label':
fs/nfsd/nfs4xdr.c:1945:15: error: 'pp' undeclared (first use in this function)
  __be32 *p = *pp;
               ^

Caused by commit 8ea0abf0a992 ("nfsd4: use xdr_reserve_space in
attribute encoding"). "This is a cosmetic change for now; no change in
behavior" :-(  They are the ones you have to be very careful of ...

I have used the nfsd tree from next-20140523 and for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Wed, May 28, 2014 at 12:07:28PM +1000, Stephen Rothwell wrote:
> Hi Bruce,
> 
> After merging the nfsd tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
> 
> 
> fs/nfsd/nfs4xdr.c: In function 'nfsd4_encode_security_label':
> fs/nfsd/nfs4xdr.c:1945:15: error: 'pp' undeclared (first use in this function)
>   __be32 *p = *pp;
>                ^
> 
> Caused by commit 8ea0abf0a992 ("nfsd4: use xdr_reserve_space in
> attribute encoding").

Fixed; thanks again.

> "This is a cosmetic change for now; no change in
> behavior" :-(  They are the ones you have to be very careful of ...

Hah, maybe.

--b.

> I have used the nfsd tree from next-20140523 and for today.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Hi Bruce,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

fs/nfsd/nfs4proc.c: In function 'warn_on_nonidempotent_op':
fs/nfsd/nfs4proc.c:1861:3: error: implicit declaration of function 'nfsd4_op_name' [-Werror=implicit-function-declaration]
   pr_err("unable to encode reply to nonidempotent op %d (%s)\n",
   ^

Caused by commit 4d332a993e64 ("nfsd4: fix encoding of out-of-space replies").

I have used the nfsd tree from next-20140523 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Mon, May 26, 2014 at 11:18:15AM +1000, Stephen Rothwell wrote:
> Hi Bruce,
> 
> After merging the nfsd tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> fs/nfsd/nfs4proc.c: In function 'warn_on_nonidempotent_op':
> fs/nfsd/nfs4proc.c:1861:3: error: implicit declaration of function 'nfsd4_op_name' [-Werror=implicit-function-declaration]
>    pr_err("unable to encode reply to nonidempotent op %d (%s)\n",
>    ^
> 
> Caused by commit 4d332a993e64 ("nfsd4: fix encoding of out-of-space replies").
> 
> I have used the nfsd tree from next-20140523 for today.

Thanks for catching that!  It should be fixed.  (But I also pushed out a
bunch of new stuff, so hopefully that didn't introduce anything equally
bad....).

--b.

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

Hi all,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

net/sunrpc/xprtsock.c:1923:30: error: 'struct rpc_task' has no member named 'tk_xprt'

Caused by commit dc107402ae06 ("SUNRPC: make AF_LOCAL connect
synchronous") interacting with commit 77102893ae68 ("SUNRPC: Nuke the
tk_xprt macro") from Linus' tree.

I have no idea how to fix this, so I have used the version of the nfsd
tree from next-20130228 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[Myklebust, Trond]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Fri, 2013-03-01 at 12:04 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the nfsd tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> net/sunrpc/xprtsock.c:1923:30: error: 'struct rpc_task' has no member named 'tk_xprt'
> 
> Caused by commit dc107402ae06 ("SUNRPC: make AF_LOCAL connect
> synchronous") interacting with commit 77102893ae68 ("SUNRPC: Nuke the
> tk_xprt macro") from Linus' tree.
> 
> I have no idea how to fix this, so I have used the version of the nfsd
> tree from next-20130228 for today.

Hi Bruce,

The attached patch should suffice to fix this up.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patches_bruce.dif --]
[-- Type: text/x-patch; name="patches_bruce.dif", Size: 471 bytes --]

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 2e7e09c..c1d8476 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1918,9 +1918,8 @@ out:
 	return status;
 }
 
-static void xs_local_connect(struct rpc_task *task)
+static void xs_local_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 {
-	struct rpc_xprt *xprt = task->tk_xprt;
 	struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
 	int ret;
 

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Fri, Mar 01, 2013 at 01:19:42AM +0000, Myklebust, Trond wrote:
> On Fri, 2013-03-01 at 12:04 +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > After merging the nfsd tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> > 
> > net/sunrpc/xprtsock.c:1923:30: error: 'struct rpc_task' has no member named 'tk_xprt'
> > 
> > Caused by commit dc107402ae06 ("SUNRPC: make AF_LOCAL connect
> > synchronous") interacting with commit 77102893ae68 ("SUNRPC: Nuke the
> > tk_xprt macro") from Linus' tree.
> > 
> > I have no idea how to fix this, so I have used the version of the nfsd
> > tree from next-20130228 for today.
> 
> Hi Bruce,
> 
> The attached patch should suffice to fix this up.

Thanks!

Looks like Linus got this right in the upstream merge.

--b.

> 
> Cheers
>   Trond
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com

> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 2e7e09c..c1d8476 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1918,9 +1918,8 @@ out:
>  	return status;
>  }
>  
> -static void xs_local_connect(struct rpc_task *task)
> +static void xs_local_connect(struct rpc_xprt *xprt, struct rpc_task *task)
>  {
> -	struct rpc_xprt *xprt = task->tk_xprt;
>  	struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
>  	int ret;
>  

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

Hi,

After merging the nfsd tree, today's linux-next build (x86_64
allmodconfig) failed like this:

fs/nfs/dns_resolve.c: In function 'nfs_dns_resolver_cache_init':
fs/nfs/dns_resolve.c:375:4: error: 'struct cache_detail' has no member named 'cache_upcall'
fs/nfs/dns_resolve.c:375:35: warning: left-hand operand of comma expression has no effect [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]

Caused by commit aab982cb5dfb ("SUNRPC: remove cache_detail->cache_upcall
callback").  Also, why don't those statements end in semicolons?

I have used the nsfd tree from next-20130128 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Sat, Feb 02, 2013 at 01:04:03PM +1100, Stephen Rothwell wrote:
> Hi,
> 
> After merging the nfsd tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> fs/nfs/dns_resolve.c: In function 'nfs_dns_resolver_cache_init':
> fs/nfs/dns_resolve.c:375:4: error: 'struct cache_detail' has no member named 'cache_upcall'
> fs/nfs/dns_resolve.c:375:35: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> 
> Caused by commit aab982cb5dfb ("SUNRPC: remove cache_detail->cache_upcall
> callback").

Yes, I knew why we'd introduced cache_upcall, so I'm not sure how I
overlooked that.  It must have slipped through testing because I didn't
set CONFIG_NFS_USE_KERNEL_DNS.

We may just be able to revert that patch....  I can take care of that by
tomorrow.

> Also, why don't those statements end in semicolons?

Looks like that whole chunk was cut-and-pasted from a previous static
initializer in 1b340d0118da1d7c60c664f17d7c8fce2bb1cd9d "NFS: DNS
resolver cache per network namespace context introduced".

> 
> I have used the nsfd tree from next-20130128 for today.

Thanks.

--b.

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Sat, Feb 02, 2013 at 07:57:19AM -0500, J. Bruce Fields wrote:
> On Sat, Feb 02, 2013 at 01:04:03PM +1100, Stephen Rothwell wrote:
> > Hi,
> > 
> > After merging the nfsd tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > fs/nfs/dns_resolve.c: In function 'nfs_dns_resolver_cache_init':
> > fs/nfs/dns_resolve.c:375:4: error: 'struct cache_detail' has no member named 'cache_upcall'
> > fs/nfs/dns_resolve.c:375:35: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> > 
> > Caused by commit aab982cb5dfb ("SUNRPC: remove cache_detail->cache_upcall
> > callback").
> 
> Yes, I knew why we'd introduced cache_upcall, so I'm not sure how I
> overlooked that.  It must have slipped through testing because I didn't
> set CONFIG_NFS_USE_KERNEL_DNS.
> 
> We may just be able to revert that patch....  I can take care of that by
> tomorrow.

Stanislav, any objections to this?

--b.

commit 8a55a995d3bada0cd0fc67be92a9e389edf709f8
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Sat Feb 2 07:49:01 2013 -0500

    Revert "SUNRPC: remove cache_detail->cache_upcall callback"
    
    This reverts commit aab982cb5dfb13405236009a6fa24807ee93f62c "SUNRPC:
    remove cache_detail->cache_upcall callback", which overlooked a
    cache_upcall user in fs/nfs/dns_resolve.c.
    
    Also note 517eb5600b455b608fb4cf38e403909a82c76100 "SUNRPC: introduce
    cache_detail->cache_request callback" missed initializing
    ->cache_request in the same file.
    
    Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
    Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index 0ff1f4c..794b237 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -373,6 +373,7 @@ int nfs_dns_resolver_cache_init(struct net *net)
 	cd->name = "dns_resolve",
 	cd->cache_put = nfs_dns_ent_put,
 	cd->cache_upcall = nfs_dns_upcall,
+	cd->cache_request = nfs_dns_request;
 	cd->cache_parse = nfs_dns_parse,
 	cd->cache_show = nfs_dns_show,
 	cd->match = nfs_dns_match,
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 30a572d..b1e367d 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -67,6 +67,11 @@ static void expkey_request(struct cache_detail *cd,
 	(*bpp)[-1] = '\n';
 }
 
+static int expkey_upcall(struct cache_detail *cd, struct cache_head *h)
+{
+	return sunrpc_cache_pipe_upcall(cd, h);
+}
+
 static struct svc_expkey *svc_expkey_update(struct cache_detail *cd, struct svc_expkey *new,
 					    struct svc_expkey *old);
 static struct svc_expkey *svc_expkey_lookup(struct cache_detail *cd, struct svc_expkey *);
@@ -240,6 +245,7 @@ static struct cache_detail svc_expkey_cache_template = {
 	.hash_size	= EXPKEY_HASHMAX,
 	.name		= "nfsd.fh",
 	.cache_put	= expkey_put,
+	.cache_upcall	= expkey_upcall,
 	.cache_request	= expkey_request,
 	.cache_parse	= expkey_parse,
 	.cache_show	= expkey_show,
@@ -333,6 +339,11 @@ static void svc_export_request(struct cache_detail *cd,
 	(*bpp)[-1] = '\n';
 }
 
+static int svc_export_upcall(struct cache_detail *cd, struct cache_head *h)
+{
+	return sunrpc_cache_pipe_upcall(cd, h);
+}
+
 static struct svc_export *svc_export_update(struct svc_export *new,
 					    struct svc_export *old);
 static struct svc_export *svc_export_lookup(struct svc_export *);
@@ -702,6 +713,7 @@ static struct cache_detail svc_export_cache_template = {
 	.hash_size	= EXPORT_HASHMAX,
 	.name		= "nfsd.export",
 	.cache_put	= svc_export_put,
+	.cache_upcall	= svc_export_upcall,
 	.cache_request	= svc_export_request,
 	.cache_parse	= svc_export_parse,
 	.cache_show	= svc_export_show,
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index d9402ea..d863cd8 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -140,6 +140,12 @@ idtoname_request(struct cache_detail *cd, struct cache_head *ch, char **bpp,
 }
 
 static int
+idtoname_upcall(struct cache_detail *cd, struct cache_head *ch)
+{
+	return sunrpc_cache_pipe_upcall(cd, ch);
+}
+
+static int
 idtoname_match(struct cache_head *ca, struct cache_head *cb)
 {
 	struct ent *a = container_of(ca, struct ent, h);
@@ -186,6 +192,7 @@ static struct cache_detail idtoname_cache_template = {
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.idtoname",
 	.cache_put	= ent_put,
+	.cache_upcall	= idtoname_upcall,
 	.cache_request	= idtoname_request,
 	.cache_parse	= idtoname_parse,
 	.cache_show	= idtoname_show,
@@ -315,6 +322,12 @@ nametoid_request(struct cache_detail *cd, struct cache_head *ch, char **bpp,
 }
 
 static int
+nametoid_upcall(struct cache_detail *cd, struct cache_head *ch)
+{
+	return sunrpc_cache_pipe_upcall(cd, ch);
+}
+
+static int
 nametoid_match(struct cache_head *ca, struct cache_head *cb)
 {
 	struct ent *a = container_of(ca, struct ent, h);
@@ -353,6 +366,7 @@ static struct cache_detail nametoid_cache_template = {
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.nametoid",
 	.cache_put	= ent_put,
+	.cache_upcall	= nametoid_upcall,
 	.cache_request	= nametoid_request,
 	.cache_parse	= nametoid_parse,
 	.cache_show	= nametoid_show,
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index ea927ea..303399b 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -80,6 +80,9 @@ struct cache_detail {
 	char			*name;
 	void			(*cache_put)(struct kref *);
 
+	int			(*cache_upcall)(struct cache_detail *,
+						struct cache_head *);
+
 	void			(*cache_request)(struct cache_detail *cd,
 						 struct cache_head *ch,
 						 char **bpp, int *blen);
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 1bd18af..3d51d03 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -182,6 +182,12 @@ static void rsi_request(struct cache_detail *cd,
 	(*bpp)[-1] = '\n';
 }
 
+static int rsi_upcall(struct cache_detail *cd, struct cache_head *h)
+{
+	return sunrpc_cache_pipe_upcall(cd, h);
+}
+
+
 static int rsi_parse(struct cache_detail *cd,
 		    char *mesg, int mlen)
 {
@@ -269,6 +275,7 @@ static struct cache_detail rsi_cache_template = {
 	.hash_size	= RSI_HASHMAX,
 	.name           = "auth.rpcsec.init",
 	.cache_put      = rsi_put,
+	.cache_upcall   = rsi_upcall,
 	.cache_request  = rsi_request,
 	.cache_parse    = rsi_parse,
 	.match		= rsi_match,
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 82f4c29..5c1ca55 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -196,9 +196,9 @@ EXPORT_SYMBOL_GPL(sunrpc_cache_update);
 
 static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h)
 {
-	if (!cd->cache_request)
+	if (!cd->cache_upcall)
 		return -EINVAL;
-	return sunrpc_cache_pipe_upcall(cd, h);
+	return cd->cache_upcall(cd, h);
 }
 
 static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h)
@@ -1609,7 +1609,7 @@ static int create_cache_proc_entries(struct cache_detail *cd, struct net *net)
 	if (p == NULL)
 		goto out_nomem;
 
-	if (cd->cache_request || cd->cache_parse) {
+	if (cd->cache_upcall || cd->cache_parse) {
 		p = proc_create_data("channel", S_IFREG|S_IRUSR|S_IWUSR,
 				     cd->u.procfs.proc_ent,
 				     &cache_file_operations_procfs, cd);
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 002ddd9..6d4944f 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -157,6 +157,11 @@ static void ip_map_request(struct cache_detail *cd,
 	(*bpp)[-1] = '\n';
 }
 
+static int ip_map_upcall(struct cache_detail *cd, struct cache_head *h)
+{
+	return sunrpc_cache_pipe_upcall(cd, h);
+}
+
 static struct ip_map *__ip_map_lookup(struct cache_detail *cd, char *class, struct in6_addr *addr);
 static int __ip_map_update(struct cache_detail *cd, struct ip_map *ipm, struct unix_domain *udom, time_t expiry);
 
@@ -465,6 +470,11 @@ static void unix_gid_request(struct cache_detail *cd,
 	(*bpp)[-1] = '\n';
 }
 
+static int unix_gid_upcall(struct cache_detail *cd, struct cache_head *h)
+{
+	return sunrpc_cache_pipe_upcall(cd, h);
+}
+
 static struct unix_gid *unix_gid_lookup(struct cache_detail *cd, uid_t uid);
 
 static int unix_gid_parse(struct cache_detail *cd,
@@ -567,6 +577,7 @@ static struct cache_detail unix_gid_cache_template = {
 	.hash_size	= GID_HASHMAX,
 	.name		= "auth.unix.gid",
 	.cache_put	= unix_gid_put,
+	.cache_upcall	= unix_gid_upcall,
 	.cache_request	= unix_gid_request,
 	.cache_parse	= unix_gid_parse,
 	.cache_show	= unix_gid_show,
@@ -864,6 +875,7 @@ static struct cache_detail ip_map_cache_template = {
 	.hash_size	= IP_HASHMAX,
 	.name		= "auth.unix.ip",
 	.cache_put	= ip_map_put,
+	.cache_upcall	= ip_map_upcall,
 	.cache_request	= ip_map_request,
 	.cache_parse	= ip_map_parse,
 	.cache_show	= ip_map_show,

Re: linux-next: build failure after merge of the nfsd tree

[Stanislav Kinsbursky]

03.02.2013 18:41, J. Bruce Fields пишет:
> On Sat, Feb 02, 2013 at 07:57:19AM -0500, J. Bruce Fields wrote:
>> On Sat, Feb 02, 2013 at 01:04:03PM +1100, Stephen Rothwell wrote:
>>> Hi,
>>>
>>> After merging the nfsd tree, today's linux-next build (x86_64
>>> allmodconfig) failed like this:
>>>
>>> fs/nfs/dns_resolve.c: In function 'nfs_dns_resolver_cache_init':
>>> fs/nfs/dns_resolve.c:375:4: error: 'struct cache_detail' has no member named 'cache_upcall'
>>> fs/nfs/dns_resolve.c:375:35: warning: left-hand operand of comma expression has no effect [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>> fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
>>>
>>> Caused by commit aab982cb5dfb ("SUNRPC: remove cache_detail->cache_upcall
>>> callback").
>>
>> Yes, I knew why we'd introduced cache_upcall, so I'm not sure how I
>> overlooked that.  It must have slipped through testing because I didn't
>> set CONFIG_NFS_USE_KERNEL_DNS.
>>
>> We may just be able to revert that patch....  I can take care of that by
>> tomorrow.
>
> Stanislav, any objections to this?
>

The only objection is that I've sent you the patch set witch fixes all these problems already:

"[PATCH v2 0/6] SUNRPC: rework cache upcall to avoid NFSd root". :)

The only reason why I removed cache_upcall at all was that all it's users (except NFS DNS cache - my mistake) just call sunrpc_cache_pipe_upcall and thus these 
wrapper looked redundant to me.
Second patch set leaves cache_upcall only for NFS DNS cache (since this upcall is not just a wrapper around sunrpc_cache_pipe_upcall).
And second patch set implies the first one will be dropped.
I can, actually, send one more (incremental  this time) patch set to fix the problem, if you wish.

But it's up to you, what to commit, Bruce.

-- 
Best regards,
Stanislav Kinsbursky

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Fri, Feb 08, 2013 at 09:41:05AM +0400, Stanislav Kinsbursky wrote:
> 03.02.2013 18:41, J. Bruce Fields пишет:
> >On Sat, Feb 02, 2013 at 07:57:19AM -0500, J. Bruce Fields wrote:
> >>On Sat, Feb 02, 2013 at 01:04:03PM +1100, Stephen Rothwell wrote:
> >>>Hi,
> >>>
> >>>After merging the nfsd tree, today's linux-next build (x86_64
> >>>allmodconfig) failed like this:
> >>>
> >>>fs/nfs/dns_resolve.c: In function 'nfs_dns_resolver_cache_init':
> >>>fs/nfs/dns_resolve.c:375:4: error: 'struct cache_detail' has no member named 'cache_upcall'
> >>>fs/nfs/dns_resolve.c:375:35: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>fs/nfs/dns_resolve.c:375:35: warning: value computed is not used [-Wunused-value]
> >>>
> >>>Caused by commit aab982cb5dfb ("SUNRPC: remove cache_detail->cache_upcall
> >>>callback").
> >>
> >>Yes, I knew why we'd introduced cache_upcall, so I'm not sure how I
> >>overlooked that.  It must have slipped through testing because I didn't
> >>set CONFIG_NFS_USE_KERNEL_DNS.
> >>
> >>We may just be able to revert that patch....  I can take care of that by
> >>tomorrow.
> >
> >Stanislav, any objections to this?
> >
> 
> The only objection is that I've sent you the patch set witch fixes all these problems already:
> 
> "[PATCH v2 0/6] SUNRPC: rework cache upcall to avoid NFSd root". :)
> 
> The only reason why I removed cache_upcall at all was that all it's
> users (except NFS DNS cache - my mistake) just call
> sunrpc_cache_pipe_upcall and thus these wrapper looked redundant to
> me.
> Second patch set leaves cache_upcall only for NFS DNS cache (since this upcall is not just a wrapper around sunrpc_cache_pipe_upcall).
> And second patch set implies the first one will be dropped.
> I can, actually, send one more (incremental  this time) patch set to fix the problem, if you wish.

No, the replacement series is fine.  I gave Trond another poke and then
they should get committed.--b.

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

Hi Bruce,

After merging the nfsd tree, today's linux-next build (x86_64
allmodconfig) failed like this:

fs/nfsd/nfs4state.c:4446:5: error: expected ';', ',' or ')' before 'void'
fs/nfsd/nfs4state.c: In function 'nfsd_forget_locks':
fs/nfsd/nfs4state.c:4468:2: error: implicit declaration of function 'nfsd_release_n_owners' [-Werror=implicit-function-declaration]

Caused by commit 16bfdaafa2c6 ("nfsd4: share open and lock owner hash
tables").  Please build test *before* releasing code ...  :-(

I have used the nfsd tree from next-20111115 for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Wed, Nov 16, 2011 at 10:29:52AM +1100, Stephen Rothwell wrote:
> Hi Bruce,
> 
> After merging the nfsd tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> fs/nfsd/nfs4state.c:4446:5: error: expected ';', ',' or ')' before 'void'
> fs/nfsd/nfs4state.c: In function 'nfsd_forget_locks':
> fs/nfsd/nfs4state.c:4468:2: error: implicit declaration of function 'nfsd_release_n_owners' [-Werror=implicit-function-declaration]
> 
> Caused by commit 16bfdaafa2c6 ("nfsd4: share open and lock owner hash
> tables").  Please build test *before* releasing code ...  :-(

I do--so what happened?

Aie, sorry, I see: we added a CONFIG_NFSD_FAULT_INJECTION recently, but
I didn't add that to my configs....

OK, I've fixed up my test scripts, and I'll push out a fix soon.

--b.

> 
> I have used the nfsd tree from next-20111115 for today.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Tue, Nov 15, 2011 at 07:14:04PM -0500, J. Bruce Fields wrote:
> On Wed, Nov 16, 2011 at 10:29:52AM +1100, Stephen Rothwell wrote:
> > Hi Bruce,
> > 
> > After merging the nfsd tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > fs/nfsd/nfs4state.c:4446:5: error: expected ';', ',' or ')' before 'void'
> > fs/nfsd/nfs4state.c: In function 'nfsd_forget_locks':
> > fs/nfsd/nfs4state.c:4468:2: error: implicit declaration of function 'nfsd_release_n_owners' [-Werror=implicit-function-declaration]
> > 
> > Caused by commit 16bfdaafa2c6 ("nfsd4: share open and lock owner hash
> > tables").  Please build test *before* releasing code ...  :-(
> 
> I do--so what happened?
> 
> Aie, sorry, I see: we added a CONFIG_NFSD_FAULT_INJECTION recently, but
> I didn't add that to my configs....
> 
> OK, I've fixed up my test scripts, and I'll push out a fix soon.

Done, thanks for the report.

--b.

linux-next: build failure after merge of the nfsd tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

Hi Bruce,

After merging the nfsd tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

ERROR: ".get_task_comm" [fs/nfsd/nfsd.ko] undefined!

Caused by commit c67874f942e30039442d925b03793e0a46ddcddd ("nfsd:
formally deprecate legacy nfsd syscall interface").

get_task_comm is not exported to modules.

I have used the version of the nfsd tree from next-20100921 for today.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Thu, Sep 23, 2010 at 11:34:29AM +1000, Stephen Rothwell wrote:
> Hi Bruce,
> 
> After merging the nfsd tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> ERROR: ".get_task_comm" [fs/nfsd/nfsd.ko] undefined!
> 
> Caused by commit c67874f942e30039442d925b03793e0a46ddcddd ("nfsd:
> formally deprecate legacy nfsd syscall interface").
> 
> get_task_comm is not exported to modules.
> 
> I have used the version of the nfsd tree from next-20100921 for today.

Oops, thanks.

It looks like a lot of places just do a

	printk("%s using deprecated interface ...", current->comm);

so maybe we can get away with just that?

--b.

Re: linux-next: build failure after merge of the nfsd tree

[Neil Brown]

On Wed, 22 Sep 2010 22:33:15 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Sep 23, 2010 at 11:34:29AM +1000, Stephen Rothwell wrote:
> > Hi Bruce,
> > 
> > After merging the nfsd tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> > 
> > ERROR: ".get_task_comm" [fs/nfsd/nfsd.ko] undefined!
> > 
> > Caused by commit c67874f942e30039442d925b03793e0a46ddcddd ("nfsd:
> > formally deprecate legacy nfsd syscall interface").
> > 
> > get_task_comm is not exported to modules.
> > 
> > I have used the version of the nfsd tree from next-20100921 for today.
> 
> Oops, thanks.
> 
> It looks like a lot of places just do a
> 
> 	printk("%s using deprecated interface ...", current->comm);
> 
> so maybe we can get away with just that?
> 
> --b.

sched.h says:

	char comm[TASK_COMM_LEN]; /* executable name excluding path
				     - access with [gs]et_task_comm (which lock
				       it with task_lock())
				     - initialized normally by setup_new_exec */

So we should lock...

But then fs/exec.c says:
void set_task_comm(struct task_struct *tsk, char *buf)
{
	task_lock(tsk);

	/*
	 * Threads may access current->comm without holding
	 * the task lock, so write the string carefully.
	 * Readers without a lock may see incomplete new
	 * names but are safe from non-terminating string reads.
	 */
     .....

So I guess we are safe to use it unlocked for informational purposes.
That first comment could do with an update, and where-ever it was that I
copied that code from can probably be simplified too.....

NeilBrown

Re: linux-next: build failure after merge of the nfsd tree

[J. Bruce Fields]

On Thu, Sep 23, 2010 at 12:51:46PM +1000, Neil Brown wrote:
> On Wed, 22 Sep 2010 22:33:15 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> sched.h says:
> 
> 	char comm[TASK_COMM_LEN]; /* executable name excluding path
> 				     - access with [gs]et_task_comm (which lock
> 				       it with task_lock())
> 				     - initialized normally by setup_new_exec */
> 
> So we should lock...
> 
> But then fs/exec.c says:
> void set_task_comm(struct task_struct *tsk, char *buf)
> {
> 	task_lock(tsk);
> 
> 	/*
> 	 * Threads may access current->comm without holding
> 	 * the task lock, so write the string carefully.
> 	 * Readers without a lock may see incomplete new
> 	 * names but are safe from non-terminating string reads.
> 	 */
>      .....
> 
> So I guess we are safe to use it unlocked for informational purposes.
> That first comment could do with an update, and where-ever it was that I
> copied that code from can probably be simplified too.....

Sounds good.

I've added a quick modular build to my test scripts too, so hopefully
that sort of problem won't slip past me next time....

--b.