2014-01-30 14:08:43

by Russell King - ARM Linux

[permalink] [raw]
Subject: NFS client broken in Linus' tip

I just booted Linus' tip (plus a few other patches to imx-drm and imx
code), and stumbled into this interesting scenario:

# touch test
touch: cannot touch `test': Operation not supported

I also tried mkdir and mknod, all result in the same error. Hard and
symlinks links are creatable.

However, I can chmod existing files and rename them. Files can also be
deleted, and the combination of this has left me without a /etc/mtab !

The machine is a iMX6 based ARM, running root-nfs, which was mounted via
ubuntu's initramfs (so not using the kernel's built-in root-nfs.)

/proc/mounts for the root mount gives:
192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
# CONFIG_NFS_SWAP is not set
# CONFIG_NFS_V4_1 is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFS_USE_LEGACY_DNS is not set
CONFIG_NFS_USE_KERNEL_DNS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_ACL_SUPPORT=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y
CONFIG_SUNRPC_GSS=y

tcpdumping, I see:

13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test"
13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory
13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>

which is the only NFS packet(s) I see which mention "test".

and stracing touch:

open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
write(2, "touch: ", 7touch: ) = 7
write(2, "cannot touch `test'", 19cannot touch `test') = 19
write(2, ": Operation not supported", 25: Operation not supported) = 25
write(2, "\n", 1
) = 1

I think it's down to this:

commit 013cdf1088d7235da9477a2375654921d9b9ba9f
Author: Christoph Hellwig <[email protected]>
Date: Fri Dec 20 05:16:53 2013 -0800

nfs: use generic posix ACL infrastructure for v3 Posix ACLs

This causes a small behaviour change in that we don't bother to set
ACLs on file creation if the mode bit can express the access permissions
fully, and thus behaving identical to local filesystems.

Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Al Viro <[email protected]>

which adds:

+ status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
+ if (status)
+ goto out;

into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
which does this:

if (!nfs_server_capable(inode, NFS_CAP_ACLS))
return ERR_PTR(-EOPNOTSUPP);

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".


2014-01-30 14:14:11

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, Jan 30, 2014 at 02:08:34PM +0000, Russell King - ARM Linux wrote:
> into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
> which does this:
>
> if (!nfs_server_capable(inode, NFS_CAP_ACLS))
> return ERR_PTR(-EOPNOTSUPP);

Does replacing that return with a

return NULL;

fix the issue for you?

2014-01-30 14:17:05

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Jan 30, 2014, at 9:08, Russell King - ARM Linux <[email protected]> wrote:

> I just booted Linus' tip (plus a few other patches to imx-drm and imx
> code), and stumbled into this interesting scenario:
>
> # touch test
> touch: cannot touch `test': Operation not supported
>
> I also tried mkdir and mknod, all result in the same error. Hard and
> symlinks links are creatable.
>
> However, I can chmod existing files and rename them. Files can also be
> deleted, and the combination of this has left me without a /etc/mtab !
>
> The machine is a iMX6 based ARM, running root-nfs, which was mounted via
> ubuntu's initramfs (so not using the kernel's built-in root-nfs.)
>
> /proc/mounts for the root mount gives:
> 192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0
>
> CONFIG_NFS_FS=y
> CONFIG_NFS_V2=y
> CONFIG_NFS_V3=y
> CONFIG_NFS_V3_ACL=y
> CONFIG_NFS_V4=y
> # CONFIG_NFS_SWAP is not set
> # CONFIG_NFS_V4_1 is not set
> CONFIG_ROOT_NFS=y
> # CONFIG_NFS_USE_LEGACY_DNS is not set
> CONFIG_NFS_USE_KERNEL_DNS=y
> # CONFIG_NFSD is not set
> CONFIG_LOCKD=y
> CONFIG_LOCKD_V4=y
> CONFIG_NFS_ACL_SUPPORT=y
> CONFIG_NFS_COMMON=y
> CONFIG_SUNRPC=y
> CONFIG_SUNRPC_GSS=y
>
> tcpdumping, I see:
>
> 13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test"
> 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory
> 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>
>
> which is the only NFS packet(s) I see which mention "test".
>
> and stracing touch:
>
> open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
> utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
> write(2, "touch: ", 7touch: ) = 7
> write(2, "cannot touch `test'", 19cannot touch `test') = 19
> write(2, ": Operation not supported", 25: Operation not supported) = 25
> write(2, "\n", 1
> ) = 1
>
> I think it's down to this:
>
> commit 013cdf1088d7235da9477a2375654921d9b9ba9f
> Author: Christoph Hellwig <[email protected]>
> Date: Fri Dec 20 05:16:53 2013 -0800
>
> nfs: use generic posix ACL infrastructure for v3 Posix ACLs
>
> This causes a small behaviour change in that we don't bother to set
> ACLs on file creation if the mode bit can express the access permissions
> fully, and thus behaving identical to local filesystems.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Al Viro <[email protected]>
>
> which adds:
>
> + status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
> + if (status)
> + goto out;

Right, this should clearly not cause nfs4_proc_create to fail if it returns EOPNOTSUPP.

> into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
> which does this:
>
> if (!nfs_server_capable(inode, NFS_CAP_ACLS))
> return ERR_PTR(-EOPNOTSUPP);

Just for completeness sake: is the server you were running against supposed to support POSIX acls?

--
Trond Myklebust
Linux NFS client maintainer

2014-01-30 14:28:05

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, Jan 30, 2014 at 06:14:05AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 30, 2014 at 02:08:34PM +0000, Russell King - ARM Linux wrote:
> > into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
> > which does this:
> >
> > if (!nfs_server_capable(inode, NFS_CAP_ACLS))
> > return ERR_PTR(-EOPNOTSUPP);
>
> Does replacing that return with a
>
> return NULL;
>
> fix the issue for you?

Yes and no. I still end up with an empty /etc/mtab, but the file now
exists. However, I can create and echo data into /etc/mtab, but it seems
that can't happen at boot time.

I also find that kerberos doesn't work - again, it's weird - I can create
the file which kerberos is trying to create in /var/tmp (which is also on
the root nfs) manually, but when the kerberos daemons try to create it,
it fails with -EOPNOTSUPP. The kerberos daemon is running as uid 0 at
that point (I don't know about the other uids/gids though since I wasn't
able to catch them in the strace.)

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-01-30 14:31:01

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, Jan 30, 2014 at 09:17:00AM -0500, Trond Myklebust wrote:
>
> On Jan 30, 2014, at 9:08, Russell King - ARM Linux <[email protected]> wrote:
>
> > I just booted Linus' tip (plus a few other patches to imx-drm and imx
> > code), and stumbled into this interesting scenario:
> >
> > # touch test
> > touch: cannot touch `test': Operation not supported
> >
> > I also tried mkdir and mknod, all result in the same error. Hard and
> > symlinks links are creatable.
> >
> > However, I can chmod existing files and rename them. Files can also be
> > deleted, and the combination of this has left me without a /etc/mtab !
> >
> > The machine is a iMX6 based ARM, running root-nfs, which was mounted via
> > ubuntu's initramfs (so not using the kernel's built-in root-nfs.)
> >
> > /proc/mounts for the root mount gives:
> > 192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0
> >
> > CONFIG_NFS_FS=y
> > CONFIG_NFS_V2=y
> > CONFIG_NFS_V3=y
> > CONFIG_NFS_V3_ACL=y
> > CONFIG_NFS_V4=y
> > # CONFIG_NFS_SWAP is not set
> > # CONFIG_NFS_V4_1 is not set
> > CONFIG_ROOT_NFS=y
> > # CONFIG_NFS_USE_LEGACY_DNS is not set
> > CONFIG_NFS_USE_KERNEL_DNS=y
> > # CONFIG_NFSD is not set
> > CONFIG_LOCKD=y
> > CONFIG_LOCKD_V4=y
> > CONFIG_NFS_ACL_SUPPORT=y
> > CONFIG_NFS_COMMON=y
> > CONFIG_SUNRPC=y
> > CONFIG_SUNRPC_GSS=y
> >
> > tcpdumping, I see:
> >
> > 13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test"
> > 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory
> > 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>
> >
> > which is the only NFS packet(s) I see which mention "test".
> >
> > and stracing touch:
> >
> > open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
> > utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
> > write(2, "touch: ", 7touch: ) = 7
> > write(2, "cannot touch `test'", 19cannot touch `test') = 19
> > write(2, ": Operation not supported", 25: Operation not supported) = 25
> > write(2, "\n", 1
> > ) = 1
> >
> > I think it's down to this:
> >
> > commit 013cdf1088d7235da9477a2375654921d9b9ba9f
> > Author: Christoph Hellwig <[email protected]>
> > Date: Fri Dec 20 05:16:53 2013 -0800
> >
> > nfs: use generic posix ACL infrastructure for v3 Posix ACLs
> >
> > This causes a small behaviour change in that we don't bother to set
> > ACLs on file creation if the mode bit can express the access permissions
> > fully, and thus behaving identical to local filesystems.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > Signed-off-by: Al Viro <[email protected]>
> >
> > which adds:
> >
> > + status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
> > + if (status)
> > + goto out;
>
> Right, this should clearly not cause nfs4_proc_create to fail if it
> returns EOPNOTSUPP.

NFS3 :)

> > into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
> > which does this:
> >
> > if (!nfs_server_capable(inode, NFS_CAP_ACLS))
> > return ERR_PTR(-EOPNOTSUPP);
>
> Just for completeness sake: is the server you were running against supposed to support POSIX acls?

The server is an old 3.1.8 kernel with this NFS config:

CONFIG_NFS_FS=m
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_FSCACHE is not set
CONFIG_NFSD=m
CONFIG_NFSD_V3=y
# CONFIG_NFSD_V3_ACL is not set
# CONFIG_NFSD_V4 is not set
CONFIG_LOCKD=m
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y

which has worked fine with NFS clients for the last 1800 odd days... until
now.

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-01-30 14:32:11

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> Yes and no. I still end up with an empty /etc/mtab, but the file now
> exists. However, I can create and echo data into /etc/mtab, but it seems
> that can't happen at boot time.

Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

I'll be out for a long weekend soon, so I have very limited time to
debug things until Monday.

2014-01-30 14:45:49

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Jan 30, 2014, at 9:30, Russell King - ARM Linux <[email protected]> wrote:

> On Thu, Jan 30, 2014 at 09:17:00AM -0500, Trond Myklebust wrote:
>>
>> On Jan 30, 2014, at 9:08, Russell King - ARM Linux <[email protected]> wrote:
>>
>>> I just booted Linus' tip (plus a few other patches to imx-drm and imx
>>> code), and stumbled into this interesting scenario:
>>>
>>> # touch test
>>> touch: cannot touch `test': Operation not supported
>>>
>>> I also tried mkdir and mknod, all result in the same error. Hard and
>>> symlinks links are creatable.
>>>
>>> However, I can chmod existing files and rename them. Files can also be
>>> deleted, and the combination of this has left me without a /etc/mtab !
>>>
>>> The machine is a iMX6 based ARM, running root-nfs, which was mounted via
>>> ubuntu's initramfs (so not using the kernel's built-in root-nfs.)
>>>
>>> /proc/mounts for the root mount gives:
>>> 192.168.1.123:/var/boot/ci / nfs rw,relatime,vers=3,rsize=65536,wsize=65536,namlen=255,hard,nolock,proto=tcp,port=2049,timeo=7,retrans=10,sec=sys,local_lock=all,addr=192.168.1.123 0 0
>>>
>>> CONFIG_NFS_FS=y
>>> CONFIG_NFS_V2=y
>>> CONFIG_NFS_V3=y
>>> CONFIG_NFS_V3_ACL=y
>>> CONFIG_NFS_V4=y
>>> # CONFIG_NFS_SWAP is not set
>>> # CONFIG_NFS_V4_1 is not set
>>> CONFIG_ROOT_NFS=y
>>> # CONFIG_NFS_USE_LEGACY_DNS is not set
>>> CONFIG_NFS_USE_KERNEL_DNS=y
>>> # CONFIG_NFSD is not set
>>> CONFIG_LOCKD=y
>>> CONFIG_LOCKD_V4=y
>>> CONFIG_NFS_ACL_SUPPORT=y
>>> CONFIG_NFS_COMMON=y
>>> CONFIG_SUNRPC=y
>>> CONFIG_SUNRPC_GSS=y
>>>
>>> tcpdumping, I see:
>>>
>>> 13:59:51.713523 IP 192.168.1.252.1341245608 > 192.168.1.123.2049: 132 lookup fh Unknown/010007011040840000000000CC238FC8FBA0475D9D9F8356B4C44166CDC38700 "test"
>>> 13:59:51.714345 IP 192.168.1.123.2049 > 192.168.1.252.1341245608: reply ok 120 lookup ERROR: No such file or directory
>>> 13:59:51.751303 IP 192.168.1.252.797 > 192.168.1.123.nfs: . ack 3381 win 2625 <nop,nop,timestamp 474136 3431312924>
>>>
>>> which is the only NFS packet(s) I see which mention "test".
>>>
>>> and stracing touch:
>>>
>>> open("test", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE, 0666) = -1 EOPNOTSUPP (Operation not supported)
>>> utimensat(AT_FDCWD, "test", NULL, 0) = -1 ENOENT (No such file or directory)
>>> write(2, "touch: ", 7touch: ) = 7
>>> write(2, "cannot touch `test'", 19cannot touch `test') = 19
>>> write(2, ": Operation not supported", 25: Operation not supported) = 25
>>> write(2, "\n", 1
>>> ) = 1
>>>
>>> I think it's down to this:
>>>
>>> commit 013cdf1088d7235da9477a2375654921d9b9ba9f
>>> Author: Christoph Hellwig <[email protected]>
>>> Date: Fri Dec 20 05:16:53 2013 -0800
>>>
>>> nfs: use generic posix ACL infrastructure for v3 Posix ACLs
>>>
>>> This causes a small behaviour change in that we don't bother to set
>>> ACLs on file creation if the mode bit can express the access permissions
>>> fully, and thus behaving identical to local filesystems.
>>>
>>> Signed-off-by: Christoph Hellwig <[email protected]>
>>> Signed-off-by: Al Viro <[email protected]>
>>>
>>> which adds:
>>>
>>> + status = posix_acl_create(dir, &sattr->ia_mode, &default_acl, &acl);
>>> + if (status)
>>> + goto out;
>>
>> Right, this should clearly not cause nfs4_proc_create to fail if it
>> returns EOPNOTSUPP.
>
> NFS3 :)

Sorry. I fat fingered that one. I intended to write nfs3_...

>>> into nfs3_proc_create(), but this ends up calling down into nfs3_get_acl(),
>>> which does this:
>>>
>>> if (!nfs_server_capable(inode, NFS_CAP_ACLS))
>>> return ERR_PTR(-EOPNOTSUPP);
>>
>> Just for completeness sake: is the server you were running against supposed to support POSIX acls?
>
> The server is an old 3.1.8 kernel with this NFS config:
>
> CONFIG_NFS_FS=m
> CONFIG_NFS_V3=y
> # CONFIG_NFS_V3_ACL is not set
> # CONFIG_NFS_V4 is not set
> # CONFIG_NFS_FSCACHE is not set
> CONFIG_NFSD=m
> CONFIG_NFSD_V3=y
> # CONFIG_NFSD_V3_ACL is not set
> # CONFIG_NFSD_V4 is not set
> CONFIG_LOCKD=m
> CONFIG_LOCKD_V4=y
> CONFIG_NFS_COMMON=y
>
> which has worked fine with NFS clients for the last 1800 odd days... until
> now.
>

OK. I?m guessing that you?re hitting the auto-probing code further down in nfs3_get_acl(), which also returns EOPNOTSUPP in those cases. Those probably need to return NULL too, then?

However, there seems to be an inconsistency in the whole API here: posix_acl_create() and posix_acl_chmod() seem to want to return ?0? both when acls are not supported and when they are not set, however posix_acl_xattr_get() wants to return EOPNOTSUPP in the first case, and ENODATA in the second. How is the filesystem supposed to know what to return?

--
Trond Myklebust
Linux NFS client maintainer

2014-01-30 15:16:58

by Ezequiel Garcia

[permalink] [raw]
Subject: Root NFS panicing on Linus' tip (Re: NFS client broken in Linus' tip)

Hi Russell, Trond:

On Thu, Jan 30, 2014 at 02:08:34PM +0000, Russell King - ARM Linux wrote:
> I just booted Linus' tip (plus a few other patches to imx-drm and imx
> code), and stumbled into this interesting scenario:
>
[..]

> CONFIG_NFS_FS=y
> CONFIG_NFS_V2=y
> CONFIG_NFS_V3=y
> CONFIG_NFS_V3_ACL=y

Just came across another issue, but a bit more problematic, as my
kernel (Linus' tip as well) panics, after mounting the rootfs:

IP-Config: Complete:
device=eth0, hwaddr=00:50:43:50:1c:15, ipaddr=192.168.0.159, mask=255.255.255.0, gw=192.168.0.1
host=develboard, domain=, nis-domain=(none)
bootserver=192.168.0.45, rootserver=192.168.0.45, rootpath=
VFS: Mounted root (nfs filesystem) on device 0:11.
devtmpfs: mounted
Freeing unused kernel memory: 136K (c0465000 - c0487000)
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.13.0-10094-g9b0cd30 #276
task: ed839a40 ti: ed83a000 task.ti: ed83a000
PC is at xattr_resolve_name+0x14/0x94
LR is at generic_getxattr+0x2c/0x64
pc : [<c00a7ab0>] lr : [<c00a7b5c>] psr: a0000113
sp : ed83be5c ip : ed83be74 fp : ed10ebc0
r10: ed83a000 r9 : ed43d980 r8 : ed81b800
r7 : c034dad8 r6 : 00000000 r5 : c03f3dcc r4 : ed43d980
r3 : 00000014 r2 : ed83be8c r1 : ed83be74 r0 : 00000000
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 00004059 DAC: 00000015
Process swapper (pid: 1, stack limit = 0xed83a238)
Stack: (0xed83be5c to 0xed83c000)
be40: ed43d980
be60: 00000014 ed83be8c 00000000 00000000 c04bc22c c03f3dcc ed83bf14 ed43f340
be80: ed43d980 c01115cc 00000000 00000041 c04bba6c 00000000 00000000 002040d0
bea0: ed81bc00 ed10ebc0 ed81bc30 c01116f8 00000000 000004d0 ed8172d0 ed43d980
bec0: 45878fd4 00000007 bfe01007 ef7f8fc0 c04bba6c ed43d6d8 c04bba6c 00000101
bee0: 00000000 ed809fd0 ed809fc0 ed809f50 ed809f40 00000000 edb045d8 c0078bcc
bf00: ed0e5dc0 edb045d8 00000000 bf000000 ed0e5dc0 00000000 00000000 00000000
bf20: 00000000 00000000 bf000000 ed10ebc0 ed0e5dc0 00000001 edb045d8 c04926d0
bf40: ed83a000 c0492758 ed10ebc0 c008fc54 00000001 ed0e5dc0 00000002 c0090cec
bf60: c03ec85c ed0e5df4 00000000 ed839c00 c0487000 c04bcec0 c03e4f08 00000000
bf80: 00000000 00000000 00000000 00000000 00000000 c00086a8 00000000 c04bcec0
bfa0: c0344f5c c0345004 00000000 c000e398 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[<c00a7ab0>] (xattr_resolve_name) from [<00000000>] ( (null))
Code: e1a06000 e5915000 e3550000 0a00001d (e5900000)
---[ end trace 15c15b4afa9eff90 ]---
swapper (1) used greatest stack depth: 5104 bytes left
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Adding a little hack, and could produce a better strack trace.
See the diff and the stack trace below:

diff --git a/fs/xattr.c b/fs/xattr.c
index 3377dff..bd2b173 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -740,6 +740,10 @@ xattr_resolve_name(const struct xattr_handler **handlers, const char **name)

if (!*name)
return NULL;
+ if(!handlers) {
+ dump_stack();
+ panic("ouch");
+ }

for_each_xattr_handler(handlers, handler) {
const char *n = strcmp_prefix(*name, handler->prefix);

CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.13.0-10094-g9b0cd30-dirty #279
[<c0012f40>] (unwind_backtrace) from [<c00107b8>] (show_stack+0x10/0x14)
[<c00107b8>] (show_stack) from [<c00a8160>] (xattr_resolve_name+0x9c/0xa8)
[<c00a8160>] (xattr_resolve_name) from [<c00a8274>] (generic_getxattr+0x2c/0x64)
[<c00a8274>] (generic_getxattr) from [<c01115e0>] (get_vfs_caps_from_disk+0x4c/0xf4)
[<c01115e0>] (get_vfs_caps_from_disk) from [<c011170c>] (cap_bprm_set_creds+0x84/0x408)
[<c011170c>] (cap_bprm_set_creds) from [<c008fc54>] (prepare_binprm+0x80/0x11c)
[<c008fc54>] (prepare_binprm) from [<c0090cec>] (do_execve+0x33c/0x46c)
[<c0090cec>] (do_execve) from [<c00086a8>] (try_to_run_init_process+0x1c/0x50)
[<c00086a8>] (try_to_run_init_process) from [<c0345024>] (kernel_init+0xa8/0x110)
[<c0345024>] (kernel_init) from [<c000e398>] (ret_from_fork+0x14/0x3c)
Kernel panic - not syncing: ouch

FWIW, here's my piece of NFS config:

CONFIG_NFS_FS=y
CONFIG_NFS_V2=y
CONFIG_NFS_V3=y
# CONFIG_NFS_V3_ACL is not set
# CONFIG_NFS_V4 is not set
# CONFIG_NFS_SWAP is not set
CONFIG_ROOT_NFS=y
# CONFIG_NFSD is not set
CONFIG_LOCKD=y
CONFIG_LOCKD_V4=y
CONFIG_NFS_COMMON=y
CONFIG_SUNRPC=y

> I think it's down to this:
>
> commit 013cdf1088d7235da9477a2375654921d9b9ba9f
> Author: Christoph Hellwig <[email protected]>
> Date: Fri Dec 20 05:16:53 2013 -0800
>
> nfs: use generic posix ACL infrastructure for v3 Posix ACLs
>
> This causes a small behaviour change in that we don't bother to set
> ACLs on file creation if the mode bit can express the access permissions
> fully, and thus behaving identical to local filesystems.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Al Viro <[email protected]>

And also here, reverting the above seem to fix the panic.

Ideas?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

2014-01-30 15:38:23

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > exists. However, I can create and echo data into /etc/mtab, but it seems
> > that can't happen at boot time.
>
> Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?

Unfortunately, that results in some problem at boot time, which
ultimately ends up with the other three CPUs being stopped, and
hence the original reason scrolls off the screen before it can be
read... even at 1920p.

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-01-30 16:08:46

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: Root NFS panicing on Linus' tip (Re: NFS client broken in Linus' tip)

On Thu, Jan 30, 2014 at 12:17:04PM -0300, Ezequiel Garcia wrote:
> Hi Russell, Trond:
>
> On Thu, Jan 30, 2014 at 02:08:34PM +0000, Russell King - ARM Linux wrote:
> > I just booted Linus' tip (plus a few other patches to imx-drm and imx
> > code), and stumbled into this interesting scenario:
> >
> [..]
>
> > CONFIG_NFS_FS=y
> > CONFIG_NFS_V2=y
> > CONFIG_NFS_V3=y
> > CONFIG_NFS_V3_ACL=y
>
> Just came across another issue, but a bit more problematic, as my
> kernel (Linus' tip as well) panics, after mounting the rootfs:
>
> IP-Config: Complete:
> device=eth0, hwaddr=00:50:43:50:1c:15, ipaddr=192.168.0.159, mask=255.255.255.0, gw=192.168.0.1
> host=develboard, domain=, nis-domain=(none)
> bootserver=192.168.0.45, rootserver=192.168.0.45, rootpath=
> VFS: Mounted root (nfs filesystem) on device 0:11.
> devtmpfs: mounted
> Freeing unused kernel memory: 136K (c0465000 - c0487000)
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = c0004000
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.13.0-10094-g9b0cd30 #276
> task: ed839a40 ti: ed83a000 task.ti: ed83a000
> PC is at xattr_resolve_name+0x14/0x94
> LR is at generic_getxattr+0x2c/0x64
> pc : [<c00a7ab0>] lr : [<c00a7b5c>] psr: a0000113
> sp : ed83be5c ip : ed83be74 fp : ed10ebc0
> r10: ed83a000 r9 : ed43d980 r8 : ed81b800
> r7 : c034dad8 r6 : 00000000 r5 : c03f3dcc r4 : ed43d980
> r3 : 00000014 r2 : ed83be8c r1 : ed83be74 r0 : 00000000
> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 10c53c7d Table: 00004059 DAC: 00000015
> Process swapper (pid: 1, stack limit = 0xed83a238)
> Stack: (0xed83be5c to 0xed83c000)
> be40: ed43d980
> be60: 00000014 ed83be8c 00000000 00000000 c04bc22c c03f3dcc ed83bf14 ed43f340
> be80: ed43d980 c01115cc 00000000 00000041 c04bba6c 00000000 00000000 002040d0
> bea0: ed81bc00 ed10ebc0 ed81bc30 c01116f8 00000000 000004d0 ed8172d0 ed43d980
> bec0: 45878fd4 00000007 bfe01007 ef7f8fc0 c04bba6c ed43d6d8 c04bba6c 00000101
> bee0: 00000000 ed809fd0 ed809fc0 ed809f50 ed809f40 00000000 edb045d8 c0078bcc
> bf00: ed0e5dc0 edb045d8 00000000 bf000000 ed0e5dc0 00000000 00000000 00000000
> bf20: 00000000 00000000 bf000000 ed10ebc0 ed0e5dc0 00000001 edb045d8 c04926d0
> bf40: ed83a000 c0492758 ed10ebc0 c008fc54 00000001 ed0e5dc0 00000002 c0090cec
> bf60: c03ec85c ed0e5df4 00000000 ed839c00 c0487000 c04bcec0 c03e4f08 00000000
> bf80: 00000000 00000000 00000000 00000000 00000000 c00086a8 00000000 c04bcec0
> bfa0: c0344f5c c0345004 00000000 c000e398 00000000 00000000 00000000 00000000
> bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [<c00a7ab0>] (xattr_resolve_name) from [<00000000>] ( (null))
> Code: e1a06000 e5915000 e3550000 0a00001d (e5900000)
> ---[ end trace 15c15b4afa9eff90 ]---
> swapper (1) used greatest stack depth: 5104 bytes left
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> Adding a little hack, and could produce a better strack trace.
> See the diff and the stack trace below:
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 3377dff..bd2b173 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -740,6 +740,10 @@ xattr_resolve_name(const struct xattr_handler **handlers, const char **name)
>
> if (!*name)
> return NULL;
> + if(!handlers) {
> + dump_stack();
> + panic("ouch");
> + }
>
> for_each_xattr_handler(handlers, handler) {
> const char *n = strcmp_prefix(*name, handler->prefix);
>
> CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.13.0-10094-g9b0cd30-dirty #279
> [<c0012f40>] (unwind_backtrace) from [<c00107b8>] (show_stack+0x10/0x14)
> [<c00107b8>] (show_stack) from [<c00a8160>] (xattr_resolve_name+0x9c/0xa8)
> [<c00a8160>] (xattr_resolve_name) from [<c00a8274>] (generic_getxattr+0x2c/0x64)
> [<c00a8274>] (generic_getxattr) from [<c01115e0>] (get_vfs_caps_from_disk+0x4c/0xf4)
> [<c01115e0>] (get_vfs_caps_from_disk) from [<c011170c>] (cap_bprm_set_creds+0x84/0x408)
> [<c011170c>] (cap_bprm_set_creds) from [<c008fc54>] (prepare_binprm+0x80/0x11c)
> [<c008fc54>] (prepare_binprm) from [<c0090cec>] (do_execve+0x33c/0x46c)
> [<c0090cec>] (do_execve) from [<c00086a8>] (try_to_run_init_process+0x1c/0x50)
> [<c00086a8>] (try_to_run_init_process) from [<c0345024>] (kernel_init+0xa8/0x110)
> [<c0345024>] (kernel_init) from [<c000e398>] (ret_from_fork+0x14/0x3c)
> Kernel panic - not syncing: ouch
>
> FWIW, here's my piece of NFS config:
>
> CONFIG_NFS_FS=y
> CONFIG_NFS_V2=y
> CONFIG_NFS_V3=y
> # CONFIG_NFS_V3_ACL is not set
> # CONFIG_NFS_V4 is not set
> # CONFIG_NFS_SWAP is not set
> CONFIG_ROOT_NFS=y
> # CONFIG_NFSD is not set
> CONFIG_LOCKD=y
> CONFIG_LOCKD_V4=y
> CONFIG_NFS_COMMON=y
> CONFIG_SUNRPC=y
>
> > I think it's down to this:
> >
> > commit 013cdf1088d7235da9477a2375654921d9b9ba9f
> > Author: Christoph Hellwig <[email protected]>
> > Date: Fri Dec 20 05:16:53 2013 -0800
> >
> > nfs: use generic posix ACL infrastructure for v3 Posix ACLs
> >
> > This causes a small behaviour change in that we don't bother to set
> > ACLs on file creation if the mode bit can express the access permissions
> > fully, and thus behaving identical to local filesystems.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > Signed-off-by: Al Viro <[email protected]>
>
> And also here, reverting the above seem to fix the panic.

Reverting this commit with NFS3 ACLs enabled also fixes the problems I
reported.

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-01-31 20:59:34

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > that can't happen at boot time.
> >
> > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
>
> Unfortunately, that results in some problem at boot time, which
> ultimately ends up with the other three CPUs being stopped, and
> hence the original reason scrolls off the screen before it can be
> read... even at 1920p.
>
Hi Russell,

The following patch fixes the issue for me.

Cheers
Trond

8<-------------------------------------------------------------
>From 59bc20fe862bd85fcad61427e8669603e789d163 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <[email protected]>
Date: Fri, 31 Jan 2014 14:25:19 -0500
Subject: [PATCH] fs: get_acl() must be allowed to return EOPNOTSUPP

posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the
filesystem cannot support acls. This is needed for NFS, which can't
know whether or not the server supports acls until it tries to get/set
one.
This patch converts posix_acl_chmod and posix_acl_create to deal with
EOPNOTSUPP return values from get_acl().

Reported-by: Russell King <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Cc: Christoph Hellwig <[email protected]>
Cc: Al Viro [email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/posix_acl.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 38bae5a0ea25..11c54fd51e16 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -521,8 +521,11 @@ posix_acl_chmod(struct inode *inode, umode_t mode)
return -EOPNOTSUPP;

acl = get_acl(inode, ACL_TYPE_ACCESS);
- if (IS_ERR_OR_NULL(acl))
+ if (IS_ERR_OR_NULL(acl)) {
+ if (acl == ERR_PTR(-EOPNOTSUPP))
+ return 0;
return PTR_ERR(acl);
+ }

ret = __posix_acl_chmod(&acl, GFP_KERNEL, mode);
if (ret)
@@ -544,14 +547,15 @@ posix_acl_create(struct inode *dir, umode_t *mode,
goto no_acl;

p = get_acl(dir, ACL_TYPE_DEFAULT);
- if (IS_ERR(p))
+ if (IS_ERR(p)) {
+ if (p == ERR_PTR(-EOPNOTSUPP))
+ goto apply_umask;
return PTR_ERR(p);
-
- if (!p) {
- *mode &= ~current_umask();
- goto no_acl;
}

+ if (!p)
+ goto apply_umask;
+
*acl = posix_acl_clone(p, GFP_NOFS);
if (!*acl)
return -ENOMEM;
@@ -575,6 +579,8 @@ posix_acl_create(struct inode *dir, umode_t *mode,
}
return 0;

+apply_umask:
+ *mode &= ~current_umask();
no_acl:
*default_acl = NULL;
*acl = NULL;
--
1.8.5.3



--
Trond Myklebust
Linux NFS client maintainer

2014-02-01 01:03:43

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > > that can't happen at boot time.
> > >
> > > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> >
> > Unfortunately, that results in some problem at boot time, which
> > ultimately ends up with the other three CPUs being stopped, and
> > hence the original reason scrolls off the screen before it can be
> > read... even at 1920p.
> >
> Hi Russell,
>
> The following patch fixes the issue for me.

It doesn't entirely fix the issue for me, instead we've got even weirder
behaviour:

root@cubox-i4:~# ls -al test
ls: cannot access test: No such file or directory
root@cubox-i4:~# touch test
root@cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test
root@cubox-i4:~# echo foo > test
root@cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 4 Feb 1 01:01 test
root@cubox-i4:~# cat test
foo
root@cubox-i4:~# rm test
root@cubox-i4:~# echo foo > test
-bash: test: Operation not supported
root@cubox-i4:~# ls -al test
-rw-r--r-- 1 root root 0 Feb 1 01:01 test

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-02-02 12:28:11

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> > On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > > > that can't happen at boot time.
> > > >
> > > > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> > >
> > > Unfortunately, that results in some problem at boot time, which
> > > ultimately ends up with the other three CPUs being stopped, and
> > > hence the original reason scrolls off the screen before it can be
> > > read... even at 1920p.
> > >
> > Hi Russell,
> >
> > The following patch fixes the issue for me.
>
> It doesn't entirely fix the issue for me, instead we've got even weirder
> behaviour:
>
> root@cubox-i4:~# ls -al test
> ls: cannot access test: No such file or directory
> root@cubox-i4:~# touch test
> root@cubox-i4:~# ls -al test
> -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> root@cubox-i4:~# echo foo > test
> root@cubox-i4:~# ls -al test
> -rw-r--r-- 1 root root 4 Feb 1 01:01 test
> root@cubox-i4:~# cat test
> foo
> root@cubox-i4:~# rm test
> root@cubox-i4:~# echo foo > test
> -bash: test: Operation not supported
> root@cubox-i4:~# ls -al test
> -rw-r--r-- 1 root root 0 Feb 1 01:01 test

FYI, I just tested Linus' tip, and NFS is still broken.

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-02-02 22:04:42

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> > On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> > > On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > > > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > > > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > > > > that can't happen at boot time.
> > > > >
> > > > > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> > > >
> > > > Unfortunately, that results in some problem at boot time, which
> > > > ultimately ends up with the other three CPUs being stopped, and
> > > > hence the original reason scrolls off the screen before it can be
> > > > read... even at 1920p.
> > > >
> > > Hi Russell,
> > >
> > > The following patch fixes the issue for me.
> >
> > It doesn't entirely fix the issue for me, instead we've got even weirder
> > behaviour:
> >
> > root@cubox-i4:~# ls -al test
> > ls: cannot access test: No such file or directory
> > root@cubox-i4:~# touch test
> > root@cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> > root@cubox-i4:~# echo foo > test
> > root@cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 4 Feb 1 01:01 test
> > root@cubox-i4:~# cat test
> > foo
> > root@cubox-i4:~# rm test
> > root@cubox-i4:~# echo foo > test
> > -bash: test: Operation not supported
> > root@cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
>
> FYI, I just tested Linus' tip, and NFS is still broken.
>
Hi Russell,

The following patch should fix the above problem. It needs to be applied
on top of the one I sent you previously. In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/[email protected]

Cheers,
Trond

8<------------------------------------------------------------------------------
>From f9ad7a7b43554bc36eccf03d33617c05b8a2c77d Mon Sep 17 00:00:00 2001
From: Trond Myklebust <[email protected]>
Date: Sun, 2 Feb 2014 14:36:42 -0500
Subject: [PATCH] NFSv3: Fix return value of nfs3_proc_setacls

nfs3_proc_setacls is used internally by the NFSv3 create operations
to set the acl after the file has been created. If the operation
fails because the server doesn't support acls, then it must return '0',
not -EOPNOTSUPP.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs3acl.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 9271a6bb9a41..871d6eda8dba 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -113,7 +113,7 @@ getout:
return ERR_PTR(status);
}

-int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+static int __nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
struct posix_acl *dfacl)
{
struct nfs_server *server = NFS_SERVER(inode);
@@ -198,6 +198,15 @@ out:
return status;
}

+int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+ struct posix_acl *dfacl)
+{
+ int ret;
+ ret = __nfs3_proc_setacls(inode, acl, dfacl);
+ return (ret == -EOPNOTSUPP) ? 0 : ret;
+
+}
+
int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
{
struct posix_acl *alloc = NULL, *dfacl = NULL;
@@ -225,7 +234,7 @@ int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
if (IS_ERR(alloc))
goto fail;
}
- status = nfs3_proc_setacls(inode, acl, dfacl);
+ status = __nfs3_proc_setacls(inode, acl, dfacl);
posix_acl_release(alloc);
return status;

--
1.8.5.3


--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 08:03:34

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the
> filesystem cannot support acls. This is needed for NFS, which can't
> know whether or not the server supports acls until it tries to get/set
> one.
> This patch converts posix_acl_chmod and posix_acl_create to deal with
> EOPNOTSUPP return values from get_acl().

Shouldn't NFS just return a NULL ACL here?

2014-02-03 09:43:37

by Takashi Iwai

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

At Sun, 02 Feb 2014 17:04:38 -0500,
Trond Myklebust wrote:
>
> On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
> > On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> > > On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> > > > On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > > > > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > > > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > > > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > > > > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > > > > > that can't happen at boot time.
> > > > > >
> > > > > > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> > > > >
> > > > > Unfortunately, that results in some problem at boot time, which
> > > > > ultimately ends up with the other three CPUs being stopped, and
> > > > > hence the original reason scrolls off the screen before it can be
> > > > > read... even at 1920p.
> > > > >
> > > > Hi Russell,
> > > >
> > > > The following patch fixes the issue for me.
> > >
> > > It doesn't entirely fix the issue for me, instead we've got even weirder
> > > behaviour:
> > >
> > > root@cubox-i4:~# ls -al test
> > > ls: cannot access test: No such file or directory
> > > root@cubox-i4:~# touch test
> > > root@cubox-i4:~# ls -al test
> > > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> > > root@cubox-i4:~# echo foo > test
> > > root@cubox-i4:~# ls -al test
> > > -rw-r--r-- 1 root root 4 Feb 1 01:01 test
> > > root@cubox-i4:~# cat test
> > > foo
> > > root@cubox-i4:~# rm test
> > > root@cubox-i4:~# echo foo > test
> > > -bash: test: Operation not supported
> > > root@cubox-i4:~# ls -al test
> > > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> >
> > FYI, I just tested Linus' tip, and NFS is still broken.
> >
> Hi Russell,
>
> The following patch should fix the above problem. It needs to be applied
> on top of the one I sent you previously.

I've hit the same problem, and your two patches seem fixing it.
I tested them on top of 3.14-rc1. Feel free to take my tested-by tag

Tested-by: Takashi Iwai <[email protected]>


> In addition, you will want to
> apply Noah Massey's patch from
> http://lkml.kernel.org/r/[email protected]

Do I still need to test this one, too?


Thanks!

Takashi

2014-02-03 14:17:35

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Feb 3, 2014, at 3:03, Christoph Hellwig <[email protected]> wrote:

> On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
>> posix_acl_xattr_get requires get_acl() to return EOPNOTSUPP if the
>> filesystem cannot support acls. This is needed for NFS, which can't
>> know whether or not the server supports acls until it tries to get/set
>> one.
>> This patch converts posix_acl_chmod and posix_acl_create to deal with
>> EOPNOTSUPP return values from get_acl().
>
> Shouldn't NFS just return a NULL ACL here?

As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 14:21:20

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Feb 3, 2014, at 4:43, Takashi Iwai <[email protected]> wrote:

> At Sun, 02 Feb 2014 17:04:38 -0500,
> Trond Myklebust wrote:
>>
>> On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
>>> On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
>>>> On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
>>>>> On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
>>>>>> On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
>>>>>>> On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
>>>>>>>> Yes and no. I still end up with an empty /etc/mtab, but the file now
>>>>>>>> exists. However, I can create and echo data into /etc/mtab, but it seems
>>>>>>>> that can't happen at boot time.
>>>>>>>
>>>>>>> Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
>>>>>>
>>>>>> Unfortunately, that results in some problem at boot time, which
>>>>>> ultimately ends up with the other three CPUs being stopped, and
>>>>>> hence the original reason scrolls off the screen before it can be
>>>>>> read... even at 1920p.
>>>>>>
>>>>> Hi Russell,
>>>>>
>>>>> The following patch fixes the issue for me.
>>>>
>>>> It doesn't entirely fix the issue for me, instead we've got even weirder
>>>> behaviour:
>>>>
>>>> root@cubox-i4:~# ls -al test
>>>> ls: cannot access test: No such file or directory
>>>> root@cubox-i4:~# touch test
>>>> root@cubox-i4:~# ls -al test
>>>> -rw-r--r-- 1 root root 0 Feb 1 01:01 test
>>>> root@cubox-i4:~# echo foo > test
>>>> root@cubox-i4:~# ls -al test
>>>> -rw-r--r-- 1 root root 4 Feb 1 01:01 test
>>>> root@cubox-i4:~# cat test
>>>> foo
>>>> root@cubox-i4:~# rm test
>>>> root@cubox-i4:~# echo foo > test
>>>> -bash: test: Operation not supported
>>>> root@cubox-i4:~# ls -al test
>>>> -rw-r--r-- 1 root root 0 Feb 1 01:01 test
>>>
>>> FYI, I just tested Linus' tip, and NFS is still broken.
>>>
>> Hi Russell,
>>
>> The following patch should fix the above problem. It needs to be applied
>> on top of the one I sent you previously.
>
> I've hit the same problem, and your two patches seem fixing it.
> I tested them on top of 3.14-rc1. Feel free to take my tested-by tag
>
> Tested-by: Takashi Iwai <[email protected]>
>
>
>> In addition, you will want to
>> apply Noah Massey's patch from
>> http://lkml.kernel.org/r/[email protected]
>
> Do I still need to test this one, too?
>

Hi Takashi,

Noah?s patch is not a replacement for the above 2 patches; it fixes a different problem. All three patches are therefore required for a complete solution.

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 14:24:11

by Takashi Iwai

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

At Mon, 3 Feb 2014 09:21:16 -0500,
Trond Myklebust wrote:
>
>
> On Feb 3, 2014, at 4:43, Takashi Iwai <[email protected]> wrote:
>
> > At Sun, 02 Feb 2014 17:04:38 -0500,
> > Trond Myklebust wrote:
> >>
> >> On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
> >>> On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> >>>> On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> >>>>> On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> >>>>>> On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> >>>>>>> On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> >>>>>>>> Yes and no. I still end up with an empty /etc/mtab, but the file now
> >>>>>>>> exists. However, I can create and echo data into /etc/mtab, but it seems
> >>>>>>>> that can't happen at boot time.
> >>>>>>>
> >>>>>>> Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> >>>>>>
> >>>>>> Unfortunately, that results in some problem at boot time, which
> >>>>>> ultimately ends up with the other three CPUs being stopped, and
> >>>>>> hence the original reason scrolls off the screen before it can be
> >>>>>> read... even at 1920p.
> >>>>>>
> >>>>> Hi Russell,
> >>>>>
> >>>>> The following patch fixes the issue for me.
> >>>>
> >>>> It doesn't entirely fix the issue for me, instead we've got even weirder
> >>>> behaviour:
> >>>>
> >>>> root@cubox-i4:~# ls -al test
> >>>> ls: cannot access test: No such file or directory
> >>>> root@cubox-i4:~# touch test
> >>>> root@cubox-i4:~# ls -al test
> >>>> -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> >>>> root@cubox-i4:~# echo foo > test
> >>>> root@cubox-i4:~# ls -al test
> >>>> -rw-r--r-- 1 root root 4 Feb 1 01:01 test
> >>>> root@cubox-i4:~# cat test
> >>>> foo
> >>>> root@cubox-i4:~# rm test
> >>>> root@cubox-i4:~# echo foo > test
> >>>> -bash: test: Operation not supported
> >>>> root@cubox-i4:~# ls -al test
> >>>> -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> >>>
> >>> FYI, I just tested Linus' tip, and NFS is still broken.
> >>>
> >> Hi Russell,
> >>
> >> The following patch should fix the above problem. It needs to be applied
> >> on top of the one I sent you previously.
> >
> > I've hit the same problem, and your two patches seem fixing it.
> > I tested them on top of 3.14-rc1. Feel free to take my tested-by tag
> >
> > Tested-by: Takashi Iwai <[email protected]>
> >
> >
> >> In addition, you will want to
> >> apply Noah Massey's patch from
> >> http://lkml.kernel.org/r/[email protected]
> >
> > Do I still need to test this one, too?
> >
>
> Hi Takashi,
>
> Noah’s patch is not a replacement for the above 2 patches; it fixes a different problem. All three patches are therefore required for a complete solution.

OK, I'll test with Noah's patch.


thanks,

Takashi

2014-02-03 14:58:06

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer? How does userspace care wether this
server doesn't support ACLs at all or none is set? The resulting
behavior is the same.

If there's a good reason to care we might have to go with your patch,
but if we can avoid it I'd prefer to keep things simple.

2014-02-03 15:45:52

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Feb 3, 2014, at 9:57, Christoph Hellwig <[email protected]> wrote:

> On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
>> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).
>
> Is it really the wrong answer? How does userspace care wether this
> server doesn't support ACLs at all or none is set? The resulting
> behavior is the same.

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

> If there's a good reason to care we might have to go with your patch,
> but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 20:22:21

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Mon, 2014-02-03 at 10:45 -0500, Trond Myklebust wrote:
> On Feb 3, 2014, at 9:57, Christoph Hellwig <[email protected]> wrote:
>
> > On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
> >> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).
> >
> > Is it really the wrong answer? How does userspace care wether this
> > server doesn't support ACLs at all or none is set? The resulting
> > behavior is the same.
>
> It will certainly cause acl_get_file() to behave differently than previously. I’ve no idea how that will affect applications, though.
>
> > If there's a good reason to care we might have to go with your patch,
> > but if we can avoid it I'd prefer to keep things simple.
>
> One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That’s rather ugly too...

FWIW, here is the alternative patch. I've tested it, and it seems to
work.
8<---------------------------------------------------------------------
>From 2e527b12169a67e9cfcf43898ae4d15bcfa1ede9 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <[email protected]>
Date: Mon, 3 Feb 2014 13:07:05 -0500
Subject: [PATCH] NFSv3: Fix return values for get_acl()

Ensure that nfs3_get_acl() returns NULL when the server doesn't support
POSIX acls. This ensures that posix_acl_create() does the right thing,
and applies the current_umask() to the mode before returning.

Add a wrapper around posix_acl_xattr_get() so that we continue to
return EOPNOTSUPP when the server doesn't support POSIX acls. Otherwise,
the NULL return from nfs3_get_acl() will cause ENODATA to be returned.

Also add the appropriate exports to posix_acl_xattr_get, posix_acl_xattr_set
and posix_acl_xattr_list to enable their use in the wrapper.

Reported-by: Russell King <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Cc: Christoph Hellwig <[email protected]>
Cc: Al Viro [email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs3acl.c | 42 ++++++++++++++++++++++++++++++++++++-----
fs/posix_acl.c | 9 ++++++---
include/linux/posix_acl_xattr.h | 8 ++++++++
3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 871d6eda8dba..d1bc84f22f64 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -28,8 +28,10 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
};
int status, count;

- if (!nfs_server_capable(inode, NFS_CAP_ACLS))
- return ERR_PTR(-EOPNOTSUPP);
+ if (!nfs_server_capable(inode, NFS_CAP_ACLS)) {
+ cache_no_acl(inode);
+ return NULL;
+ }

status = nfs_revalidate_inode(server, inode);
if (status < 0)
@@ -70,7 +72,7 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
dprintk("NFS_V3_ACL extension not supported; disabling\n");
server->caps &= ~NFS_CAP_ACLS;
case -ENOTSUPP:
- status = -EOPNOTSUPP;
+ status = 0;
default:
goto getout;
}
@@ -242,8 +244,38 @@ fail:
return PTR_ERR(alloc);
}

+static int
+nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name,
+ void *value, size_t size, int type)
+{
+ int ret;
+
+ ret = posix_acl_xattr_get(dentry, name, value, size, type);
+ /*
+ * This check is needed to override the ENODATA error that
+ * posix_acl_xattr_get will return if the acl probe fails.
+ */
+ if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS))
+ ret = -EOPNOTSUPP;
+ return ret;
+}
+
+static const struct xattr_handler nfs_posix_acl_access_xattr_handler = {
+ .prefix = POSIX_ACL_XATTR_ACCESS,
+ .flags = ACL_TYPE_ACCESS,
+ .get = nfs_posix_acl_xattr_get,
+ .set = posix_acl_xattr_set,
+};
+
+static const struct xattr_handler nfs_posix_acl_default_xattr_handler = {
+ .prefix = POSIX_ACL_XATTR_DEFAULT,
+ .flags = ACL_TYPE_DEFAULT,
+ .get = nfs_posix_acl_xattr_get,
+ .set = posix_acl_xattr_set,
+};
+
const struct xattr_handler *nfs3_xattr_handlers[] = {
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
+ &nfs_posix_acl_access_xattr_handler,
+ &nfs_posix_acl_default_xattr_handler,
NULL,
};
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 38bae5a0ea25..835167f92952 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -750,7 +750,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
}
EXPORT_SYMBOL (posix_acl_to_xattr);

-static int
+int
posix_acl_xattr_get(struct dentry *dentry, const char *name,
void *value, size_t size, int type)
{
@@ -773,8 +773,9 @@ posix_acl_xattr_get(struct dentry *dentry, const char *name,

return error;
}
+EXPORT_SYMBOL_GPL(posix_acl_xattr_get);

-static int
+int
posix_acl_xattr_set(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags, int type)
{
@@ -809,8 +810,9 @@ out:
posix_acl_release(acl);
return ret;
}
+EXPORT_SYMBOL_GPL(posix_acl_xattr_set);

-static size_t
+size_t
posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size,
const char *name, size_t name_len, int type)
{
@@ -832,6 +834,7 @@ posix_acl_xattr_list(struct dentry *dentry, char *list, size_t list_size,
memcpy(list, xname, size);
return size;
}
+EXPORT_SYMBOL_GPL(posix_acl_xattr_list);

const struct xattr_handler posix_acl_access_xattr_handler = {
.prefix = POSIX_ACL_XATTR_ACCESS,
diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
index 6f14ee295822..fc062003a456 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -69,6 +69,14 @@ struct posix_acl *posix_acl_from_xattr(struct user_namespace *user_ns,
int posix_acl_to_xattr(struct user_namespace *user_ns,
const struct posix_acl *acl, void *buffer, size_t size);

+int posix_acl_xattr_get(struct dentry *dentry, const char *name,
+ void *value, size_t size, int type);
+int posix_acl_xattr_set(struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags, int type);
+size_t posix_acl_xattr_list(struct dentry *dentry, char *list,
+ size_t list_size, const char *name,
+ size_t name_len, int type);
+
extern const struct xattr_handler posix_acl_access_xattr_handler;
extern const struct xattr_handler posix_acl_default_xattr_handler;

--
1.8.5.3


--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 20:25:40

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Mon, Feb 03, 2014 at 03:22:15PM -0500, Trond Myklebust wrote:
> On Mon, 2014-02-03 at 10:45 -0500, Trond Myklebust wrote:
> > On Feb 3, 2014, at 9:57, Christoph Hellwig <[email protected]> wrote:
> >
> > > On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
> > >> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).
> > >
> > > Is it really the wrong answer? How does userspace care wether this
> > > server doesn't support ACLs at all or none is set? The resulting
> > > behavior is the same.
> >
> > It will certainly cause acl_get_file() to behave differently than previously. I’ve no idea how that will affect applications, though.
> >
> > > If there's a good reason to care we might have to go with your patch,
> > > but if we can avoid it I'd prefer to keep things simple.
> >
> > One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That’s rather ugly too...
>
> FWIW, here is the alternative patch. I've tested it, and it seems to
> work.

Thanks.

As there's now two fixes, which would you like me to test?

One comment on this patch though:

> +static int
> +nfs_posix_acl_xattr_get(struct dentry *dentry, const char *name,
> + void *value, size_t size, int type)
> +{
> + int ret;
> +
> + ret = posix_acl_xattr_get(dentry, name, value, size, type);
> + /*
> + * This check is needed to override the ENODATA error that
> + * posix_acl_xattr_get will return if the acl probe fails.
> + */
> + if (!nfs_server_capable(dentry->d_inode, NFS_CAP_ACLS))
> + ret = -EOPNOTSUPP;

I'm not familiar with this code, but the above looks slightly weird,
and a little suspicious - especially with the lack of blank line
before the comment. Is the above actually intended?

--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

2014-02-03 20:25:54

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

On Mon, Feb 03, 2014 at 03:22:15PM -0500, Trond Myklebust wrote:
> FWIW, here is the alternative patch. I've tested it, and it seems to
> work.

I much prefer the original one. One major point of the series was to
get individual filesystems out of the business of providing xattr
handlers for ACLs.

2014-02-03 20:32:43

by Trond Myklebust

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip


On Feb 3, 2014, at 15:25, Christoph Hellwig <[email protected]> wrote:

> On Mon, Feb 03, 2014 at 03:22:15PM -0500, Trond Myklebust wrote:
>> FWIW, here is the alternative patch. I've tested it, and it seems to
>> work.
>
> I much prefer the original one. One major point of the series was to
> get individual filesystems out of the business of providing xattr
> handlers for ACLs.
>

Then could you and Al please provide an Acked-by? This is a userspace API, so we shouldn?t be changing return values without good reason.

--
Trond Myklebust
Linux NFS client maintainer

2014-02-03 20:54:41

by Christoph Hellwig

[permalink] [raw]
Subject: Re: NFS client broken in Linus' tip

Looks good as the lesser evil:

Reviewed-by: Christoph Hellwig <[email protected]>