XATTR bugs

XATTR bugs

[Fredrik Tolf]

Hi!

I just applied the ReiserFS xattr patches to my 2.6.0-test10 Linux
kernel, and I found what appears to be two bugs.

The patches were those from
ftp://ftp.suse.com/pub/people/jeffm/reiserfs/aclea/

Since there is no mention of this URL on the namesys.com home page, I
don't know if these patches are "official". If they aren't, please
tell me so and I will turn elsewhere, and if you know where to mail it
instead, please tell me that too.

Anyway, I'm going to ramble on as if they were official.

The first bug was in inode.c and was really simple. The only thing was
that #include <linux/quotaops.h> was missing. It is necessary for
DQUOT_TRANSFER, which is used by the xattr patch, and as such the
module won't load.

The second one was a bit more subtle. I'm using this ReiserFS to serve
NFS, and I discovered that there was an ACL-related bug in
reiserfs_setattr (also from inode.c). In the end of that function
(line 2464 in my kernel), it makes a call to reiserfs_acl_chmod, like
this:

if (!error && reiserfs_posixacl (inode->i_sb)) {
    if (attr->ia_valid & ATTR_MODE)
        error = reiserfs_acl_chmod (inode);
}

reiserfs_acl_chmod always returns EOPNOTSUPP if called on a
symlink. That isn't normally a problem, but it seems that nfsd always
chmods symlinks when they are created via this method. The symlink
gets created as it should, but the RPC returns EIO (NFS apparently has
no return code for EOPNOTSUPP and therefore translates it into EIO),
which brakes many programs on the clients.

I solved it by simply not calling reiserfs_acl_chmod in case the inode
is a symlink, like this:

if (!error && !S_ISLNK(inode->i_mode) && reiserfs_posixacl (inode->i_sb)) {
    if (attr->ia_valid & ATTR_MODE)
        error = reiserfs_acl_chmod (inode);
}

The thing is that I don't know ReiserFS, and so I don't really know
what I'm doing. If you think this breaks anything, please tell me
so. In any other case, I guess it would be a good thing to change the
patches into this.

Fredrik Tolf

Re: XATTR bugs

[Hans Reiser]

Fredrik Tolf wrote:

>Hi!
>
>I just applied the ReiserFS xattr patches to my 2.6.0-test10 Linux
>kernel, and I found what appears to be two bugs.
>
>The patches were those from
>ftp://ftp.suse.com/pub/people/jeffm/reiserfs/aclea/
>
>Since there is no mention of this URL on the namesys.com home page, I
>don't know if these patches are "official".
>
Nope.  They are suse kernel only.  The official kernel has V4 as the 
planned point of entry for ACLs, and V3 is in something very close to a 
frozen state. 

> If they aren't, please
>tell me so and I will turn elsewhere, and if you know where to mail it
>instead, please tell me that too.
>  
>
jeffm@suse.de can help you.  Please DO consider them on-topic for 
reiserfs-list though.  Just because I am not accepting them into the 
official kernel does not mean I want to stand in the way of people doing 
what they want to do with our code, and doing it effectively, it just 
means I have a different philosophy of release management.;-)

>Anyway, I'm going to ramble on as if they were official.
>
>The first bug was in inode.c and was really simple. The only thing was
>that #include <linux/quotaops.h> was missing. It is necessary for
>DQUOT_TRANSFER, which is used by the xattr patch, and as such the
>module won't load.
>
>The second one was a bit more subtle. I'm using this ReiserFS to serve
>NFS, and I discovered that there was an ACL-related bug in
>reiserfs_setattr (also from inode.c). In the end of that function
>(line 2464 in my kernel), it makes a call to reiserfs_acl_chmod, like
>this:
>
>if (!error && reiserfs_posixacl (inode->i_sb)) {
>    if (attr->ia_valid & ATTR_MODE)
>        error = reiserfs_acl_chmod (inode);
>}
>
>reiserfs_acl_chmod always returns EOPNOTSUPP if called on a
>symlink. That isn't normally a problem, but it seems that nfsd always
>chmods symlinks when they are created via this method. The symlink
>gets created as it should, but the RPC returns EIO (NFS apparently has
>no return code for EOPNOTSUPP and therefore translates it into EIO),
>which brakes many programs on the clients.
>
>I solved it by simply not calling reiserfs_acl_chmod in case the inode
>is a symlink, like this:
>
>if (!error && !S_ISLNK(inode->i_mode) && reiserfs_posixacl (inode->i_sb)) {
>    if (attr->ia_valid & ATTR_MODE)
>        error = reiserfs_acl_chmod (inode);
>}
>
>The thing is that I don't know ReiserFS, and so I don't really know
>what I'm doing. If you think this breaks anything, please tell me
>so. In any other case, I guess it would be a good thing to change the
>patches into this.
>
>Fredrik Tolf
>
>
>
>
>  
>


-- 
Hans

Re: XATTR bugs

[Fredrik Tolf]

Hans Reiser writes:
 > Fredrik Tolf wrote:
 > 
 > >Hi!
 > >
 > >I just applied the ReiserFS xattr patches to my 2.6.0-test10 Linux
 > >kernel, and I found what appears to be two bugs.
 > >
 > >The patches were those from
 > >ftp://ftp.suse.com/pub/people/jeffm/reiserfs/aclea/
 > >
 > >Since there is no mention of this URL on the namesys.com home page, I
 > >don't know if these patches are "official".
 > >
 > Nope.  They are suse kernel only.  The official kernel has V4 as the 
 > planned point of entry for ACLs, and V3 is in something very close to a 
 > frozen state. 
 > 
 > > If they aren't, please
 > >tell me so and I will turn elsewhere, and if you know where to mail it
 > >instead, please tell me that too.
 > >  
 > >
 > jeffm@suse.de can help you.  Please DO consider them on-topic for 
 > reiserfs-list though.  Just because I am not accepting them into the 
 > official kernel does not mean I want to stand in the way of people doing 
 > what they want to do with our code, and doing it effectively, it just 
 > means I have a different philosophy of release management.;-)

OK, I see - I will forward my thoughts to him, then. Thanks!

Fredrik Tolf

Re: XATTR bugs

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Fredrik Tolf wrote:
| Hi!
|
| I just applied the ReiserFS xattr patches to my 2.6.0-test10 Linux
| kernel, and I found what appears to be two bugs.
|
| The patches were those from
| ftp://ftp.suse.com/pub/people/jeffm/reiserfs/aclea/
|
| Since there is no mention of this URL on the namesys.com home page, I
| don't know if these patches are "official". If they aren't, please
| tell me so and I will turn elsewhere, and if you know where to mail it
| instead, please tell me that too.
|
| Anyway, I'm going to ramble on as if they were official.

As much as I'd like them to see "official" testing, Hans is very much
against these patches against a V3 filesystem.

I've just posted updates to the patches on my (the above) FTP site.

| The first bug was in inode.c and was really simple. The only thing was
| that #include <linux/quotaops.h> was missing. It is necessary for
| DQUOT_TRANSFER, which is used by the xattr patch, and as such the
| module won't load.

This bug was fixed in the updated patches.

| The second one was a bit more subtle. I'm using this ReiserFS to serve
| NFS, and I discovered that there was an ACL-related bug in
| reiserfs_setattr (also from inode.c). In the end of that function
| (line 2464 in my kernel), it makes a call to reiserfs_acl_chmod, like
| this:
[...]
| The thing is that I don't know ReiserFS, and so I don't really know
| what I'm doing. If you think this breaks anything, please tell me
| so. In any other case, I guess it would be a good thing to change the
| patches into this.

This bug, however, I don't believe is a bug in my code. This code works
fine on 2.4 kernels, and after a few minutes of searching, I've found
the difference.

This follows behavior on ext[23], which is part of the "official" 2.6
kernel. The reason for this is as follows:

fs/nfsd/vfs.c:nfsd_setattr() is responsible for NFS server setattr()
behavior on both 2.4 and 2.6 kernels.
The following chunk of code handles the chmod:
~        if (!check_guard || guardtime == inode->i_ctime) {
~                err = notify_change(dentry, iap);
~                err = nfserrno(err);
~        }

The culprit as to why you're seeing this behavior is in nfserrno().

On v2.4 kernels, the structure that handles the error mapping ends like
this:
~                { nfserr_stale, -ESTALE },
~                { nfserr_dropit, -ENOMEM },
~                { nfserr_notsupp, -EOPNOTSUPP },
~                { nfserr_notsupp, -ENOTSUPP },
~                { -1, -EIO }

On v2.6 kernels, the structure that handles the error mapping ends like
this:
~                { nfserr_stale, -ESTALE },
~                { nfserr_dropit, -EAGAIN },
~                { nfserr_dropit, -ENOMEM },
~                { -1, -EIO }

... thus the -EOPNOTSUPP isn't mapped to a non-fatal error, and defaults
to -EIO.

The problem needs to be addressed there, not in the ReiserFS code. I'll
check out when and why that bit of code became errnoneous in the 2.6
kernel later this evening; I'm headed out the door at the moment.

- -Jeff

- --
Jeff Mahoney
SuSE Labs
jeffm@suse.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQE/1ltXLPWxlyuTD7IRAtJ7AJ9W4anIDLlZB62cF4s62btfXdiVNACdEgh3
a/kF/6BytHVd4fOBr3VoY4o=
=ZInn
-----END PGP SIGNATURE-----

Re: XATTR bugs

[Fredrik Tolf]

Jeff Mahoney writes:
 > | The first bug was in inode.c and was really simple. The only thing was
 > | that #include <linux/quotaops.h> was missing. It is necessary for
 > | DQUOT_TRANSFER, which is used by the xattr patch, and as such the
 > | module won't load.
 > 
 > This bug was fixed in the updated patches.
 > 
 > | The second one was a bit more subtle. I'm using this ReiserFS to serve
 > | NFS, and I discovered that there was an ACL-related bug in
 > | reiserfs_setattr (also from inode.c). In the end of that function
 > | (line 2464 in my kernel), it makes a call to reiserfs_acl_chmod, like
 > | this:
 > [...]
 > | The thing is that I don't know ReiserFS, and so I don't really know
 > | what I'm doing. If you think this breaks anything, please tell me
 > | so. In any other case, I guess it would be a good thing to change the
 > | patches into this.
 > 
 > This bug, however, I don't believe is a bug in my code. This code works
 > fine on 2.4 kernels, and after a few minutes of searching, I've found
 > the difference.
[...]
 > 
 > The problem needs to be addressed there, not in the ReiserFS code. I'll
 > check out when and why that bit of code became errnoneous in the 2.6
 > kernel later this evening; I'm headed out the door at the moment.

These are not the greatest problems with the XATTR patches,
though; they were only what I discovered initially. The greatest
problem (I don't know if this might not appear on 2.4 kernels, but it
certainly did occur on all my 2.6 kernels) is that ACLs can suddenly
"disappear".

I don't know exactly what it was that happened, but sometimes (without
any pattern that I could notice), the ACLs would simply become
ineffective. The XATTR for the ACLs was still there, but it was as if
it wasn't taken into account for any kind of operation. When checking
with getfattr, the ACL attribute was still there and looked just as it
did before whatever happened happened, but in every other aspect it
was as if it wasn't there anymore. The only way to solve it was to run
"setfacl -b" on the affected file and then recreate the ACL.

In fact, this forced me to abandon ReiserFS in favor of XFS.
Regrettable as it may be, I didn't want to live without ACLs, and I
had too much on my hands to start debugging it... :-(

Fredrik Tolf

Re: XATTR bugs

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Fredrik Tolf wrote:
| These are not the greatest problems with the XATTR patches,
| though; they were only what I discovered initially. The greatest
| problem (I don't know if this might not appear on 2.4 kernels, but it
| certainly did occur on all my 2.6 kernels) is that ACLs can suddenly
| "disappear".
|
| I don't know exactly what it was that happened, but sometimes (without
| any pattern that I could notice), the ACLs would simply become
| ineffective. The XATTR for the ACLs was still there, but it was as if
| it wasn't taken into account for any kind of operation. When checking
| with getfattr, the ACL attribute was still there and looked just as it
| did before whatever happened happened, but in every other aspect it
| was as if it wasn't there anymore. The only way to solve it was to run
| "setfacl -b" on the affected file and then recreate the ACL.
|
| In fact, this forced me to abandon ReiserFS in favor of XFS.
| Regrettable as it may be, I didn't want to live without ACLs, and I
| had too much on my hands to start debugging it... :-(

This is the first I've heard of this behavior. Do you have a reliable
way of reproducing it? I realize that you said you didn't find a
pattern, but what was the workload you were applying to the ACLs?

Thanks.

- -Jeff

- --
Jeff Mahoney
SuSE Labs
jeffm@suse.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQE/1oCmLPWxlyuTD7IRAopKAJ0V7DIfSG833p53CTSStnVwOOZJtgCgmDah
GpNUM39k2JSRvpFoEqCcRIQ=
=7suS
-----END PGP SIGNATURE-----

Re: XATTR bugs

[Fredrik Tolf]

Jeff Mahoney writes:
 > Fredrik Tolf wrote:
 > | These are not the greatest problems with the XATTR patches,
 > | though; they were only what I discovered initially. The greatest
 > | problem (I don't know if this might not appear on 2.4 kernels, but it
 > | certainly did occur on all my 2.6 kernels) is that ACLs can suddenly
 > | "disappear".
 > |
 > | I don't know exactly what it was that happened, but sometimes (without
 > | any pattern that I could notice), the ACLs would simply become
 > | ineffective. The XATTR for the ACLs was still there, but it was as if
 > | it wasn't taken into account for any kind of operation. When checking
 > | with getfattr, the ACL attribute was still there and looked just as it
 > | did before whatever happened happened, but in every other aspect it
 > | was as if it wasn't there anymore. The only way to solve it was to run
 > | "setfacl -b" on the affected file and then recreate the ACL.
 > |
 > | In fact, this forced me to abandon ReiserFS in favor of XFS.
 > | Regrettable as it may be, I didn't want to live without ACLs, and I
 > | had too much on my hands to start debugging it... :-(
 > 
 > This is the first I've heard of this behavior. Do you have a reliable
 > way of reproducing it? I realize that you said you didn't find a
 > pattern, but what was the workload you were applying to the ACLs?

Well, the machine I applied the patches to is the NFS server that I
use at home, so the workload isn't too great. It serves my and my
family's workstations, the web server (around 1000-2000 hits/day), and
some misc services. So as you can see, the load isn't too great, but
it's seldom completely idle.

The strange thing is that the behavior didn't seem very related to the
current workload, but rather localized to certain files/dirs. For
example, I use ACLs on all home directories to allow apache
through. Some home directories would never once lose the ACLs, while
others kept losing them all the time.

Unfortunately, I could never find a way to reproduce it manually. That
was the primary reason why I never started to debug it. At least I
know almost completely certain that it is specific to ReiserFS,
because since I switched to XFS, it hasn't happened once.

It would be really nice if you could find the bug. I still have some
time before the data grows so large that I don't have enough spare
hard drives to transfer the filesystem back to Reiser (and I really
would prefer using ReiserFS).

I'll just clarify one detail. The attribute was still there, even
though it seemed unused. Ie., a sample affected line of output from an
xattr-patched ls could look like this:

drwx------+  17 barbara  users        4096 Dec  4 21:07 barbara

As you can see, it acknowledges the existence of the attribute, but
even the mode is wrong. 700 is the mode that actually is set on the
file, but since the ACL is there (the ACL is to let apache through,
ie. u:apache:x), the reported mode should be 710. getfattr also
reports a correct ACL (ie. it hasn't been modified or corrupted or
anything).

Fredrik Tolf