Skip to content

Commit

Permalink
Btrfs: fix acl caching
Browse files Browse the repository at this point in the history
Linus noticed the btrfs code to cache acls wasn't properly caching
a NULL acl when the inode didn't have any acls.  This meant the common
case of no acls resulted in expensive btree searches every time the
kernel checked permissions (which is quite often).

This is a modified version of Linus' original patch:

Properly set initial acl fields to BTRFS_ACL_NOT_CACHED in the inode.
This forces an acl lookup when permission checks are done.

Fix btrfs_get_acl to avoid lookups and locking when the inode acls fields
are set to null.

Fix btrfs_get_acl to use the right return value from __btrfs_getxattr
when deciding to cache a NULL acl.  It was storing a NULL acl when
__btrfs_getxattr return -ENOENT, but __btrfs_getxattr was actually returning
-ENODATA for this case.

Signed-off-by: Chris Mason <chris.mason@oracle.com>
  • Loading branch information
Chris Mason committed Apr 27, 2009
1 parent 2138093 commit 7b1a14b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
18 changes: 13 additions & 5 deletions fs/btrfs/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,20 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
return ERR_PTR(-EINVAL);
}

/* Handle the cached NULL acl case without locking */
acl = ACCESS_ONCE(*p_acl);
if (!acl)
return acl;

spin_lock(&inode->i_lock);
if (*p_acl != BTRFS_ACL_NOT_CACHED)
acl = posix_acl_dup(*p_acl);
acl = *p_acl;
if (acl != BTRFS_ACL_NOT_CACHED)
acl = posix_acl_dup(acl);
spin_unlock(&inode->i_lock);

if (acl)
if (acl != BTRFS_ACL_NOT_CACHED)
return acl;


size = __btrfs_getxattr(inode, name, "", 0);
if (size > 0) {
value = kzalloc(size, GFP_NOFS);
Expand All @@ -80,9 +85,12 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
btrfs_update_cached_acl(inode, p_acl, acl);
}
kfree(value);
} else if (size == -ENOENT) {
} else if (size == -ENOENT || size == -ENODATA || size == 0) {
/* FIXME, who returns -ENOENT? I think nobody */
acl = NULL;
btrfs_update_cached_acl(inode, p_acl, acl);
} else {
acl = ERR_PTR(-EIO);
}

return acl;
Expand Down
4 changes: 2 additions & 2 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -3047,8 +3047,8 @@ static noinline void init_btrfs_i(struct inode *inode)
{
struct btrfs_inode *bi = BTRFS_I(inode);

bi->i_acl = NULL;
bi->i_default_acl = NULL;
bi->i_acl = BTRFS_ACL_NOT_CACHED;
bi->i_default_acl = BTRFS_ACL_NOT_CACHED;

bi->generation = 0;
bi->sequence = 0;
Expand Down

0 comments on commit 7b1a14b

Please sign in to comment.