xfs
[Top] [All Lists]

Re: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes

To: Andreas Gruenbacher <agruenba@xxxxxxxxxx>, Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes
From: Jeff Layton <jlayton@xxxxxxxxxx>
Date: Tue, 05 Jul 2016 11:56:03 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Theodore Ts'o <tytso@xxxxxxx>, Andreas Dilger <adilger.kernel@xxxxxxxxx>, "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>, Anna Schumaker <anna.schumaker@xxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, linux-ext4@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, linux-cifs@xxxxxxxxxxxxxxx, linux-api@xxxxxxxxxxxxxxx, Andreas Dilger <adilger@xxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1467294433-3222-12-git-send-email-agruenba@xxxxxxxxxx>
References: <1467294433-3222-1-git-send-email-agruenba@xxxxxxxxxx> <1467294433-3222-12-git-send-email-agruenba@xxxxxxxxxx>
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
> POSIX ACLs and richacls are both objects allocated by kmalloc() with a
> reference count which are freed by kfree_rcu().ÂÂAn inode can either
> cache an access and a default POSIX ACL, or a richacl (richacls do not
> have default acls).ÂÂTo allow an inode to cache either of the two kinds
> of acls, introduce a new base_acl type and convert i_acl and
> i_default_acl to that type. In most cases, the vfs then doesn't care which
> kind of acl an inode caches (if any).
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> Cc: Andreas Dilger <adilger@xxxxxxxxx>
> ---
> Âdrivers/staging/lustre/lustre/llite/llite_lib.c |ÂÂ2 +-
> Âfs/9p/acl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ8 +--
> Âfs/f2fs/acl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ4 +-
> Âfs/inode.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 32 +++++++++++-
> Âfs/jffs2/acl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ6 ++-
> Âfs/namei.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 33 ++++++------
> Âfs/nfs/nfs3acl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 14 ++---
> Âfs/posix_acl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 69 
> +++++++------------------
> Âfs/richacl.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ4 +-
> Âinclude/linux/fs.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 41 +++++++++++++--
> Âinclude/linux/posix_acl.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 21 ++++----
> Âinclude/linux/richacl.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ9 ++--
> Â12 files changed, 139 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c 
> b/drivers/staging/lustre/lustre/llite/llite_lib.c
> index 96c7e9f..7819a7c 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_lib.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
> @@ -1087,7 +1087,7 @@ void ll_clear_inode(struct inode *inode)
> Â     }
> Â#ifdef CONFIG_FS_POSIX_ACL
> Â     else if (lli->lli_posix_acl) {
> -             LASSERT(atomic_read(&lli->lli_posix_acl->a_refcount) == 1);
> +             LASSERT(base_acl_refcount(&lli->lli_posix_acl->a_base) == 1);
> Â             LASSERT(!lli->lli_remote_perms);
> Â             posix_acl_release(lli->lli_posix_acl);
> Â             lli->lli_posix_acl = NULL;
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index 0576eae..1ce572f 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -87,14 +87,14 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
> Â
> Âstatic struct posix_acl *v9fs_get_cached_acl(struct inode *inode, int type)
> Â{
> -     struct posix_acl *acl;
> +     struct base_acl *base_acl;
> Â     /*
> Â     Â* 9p Always cache the acl value when
> Â     Â* instantiating the inode (v9fs_inode_from_fid)
> Â     Â*/
> -     acl = get_cached_acl(inode, type);
> -     BUG_ON(is_uncached_acl(acl));
> -     return acl;
> +     base_acl = get_cached_acl(inode, type);
> +     BUG_ON(is_uncached_acl(base_acl));
> +     return posix_acl(base_acl);
> Â}
> Â
> Âstruct posix_acl *v9fs_iop_get_acl(struct inode *inode, int type)
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index a31c7e8..6079017 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -267,7 +267,7 @@ static struct posix_acl *f2fs_acl_clone(const struct 
> posix_acl *acl,
> Â                             sizeof(struct posix_acl_entry);
> Â             clone = kmemdup(acl, size, flags);
> Â             if (clone)
> -                     atomic_set(&clone->a_refcount, 1);
> +                     base_acl_init(&clone->a_base);
> Â     }
> Â     return clone;
> Â}
> @@ -279,7 +279,7 @@ static int f2fs_acl_create_masq(struct posix_acl *acl, 
> umode_t *mode_p)
> Â     umode_t mode = *mode_p;
> Â     int not_equiv = 0;
> Â
> -     /* assert(atomic_read(acl->a_refcount) == 1); */
> +     /* assert(base_acl_refcount(&acl->a_base) == 1); */
> Â
> Â     FOREACH_ACL_ENTRY(pa, acl, pe) {
> Â             switch(pa->e_tag) {
> diff --git a/fs/inode.c b/fs/inode.c
> index 4ccbc21..40c03a7 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -240,14 +240,42 @@ void __destroy_inode(struct inode *inode)
> Â
> Â#ifdef CONFIG_FS_POSIX_ACL
> Â     if (inode->i_acl && !is_uncached_acl(inode->i_acl))
> -             posix_acl_release(inode->i_acl);
> +             base_acl_put(inode->i_acl);
> Â     if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl))
> -             posix_acl_release(inode->i_default_acl);
> +             base_acl_put(inode->i_default_acl);
> Â#endif
> Â     this_cpu_dec(nr_inodes);
> Â}
> ÂEXPORT_SYMBOL(__destroy_inode);
> Â
> +#ifdef CONFIG_FS_POSIX_ACL
> +struct base_acl *__get_cached_acl(struct base_acl **p)
> +{
> +     struct base_acl *base_acl;
> +
> +     for (;;) {
> +             rcu_read_lock();
> +             base_acl = rcu_dereference(*p);
> +             if (!base_acl || is_uncached_acl(base_acl) ||
> +             ÂÂÂÂatomic_inc_not_zero(&base_acl->ba_refcount))
> +                     break;
> +             rcu_read_unlock();
> +             cpu_relax();
> +     }
> +     rcu_read_unlock();
> +     return base_acl;
> +}
> +

I know this is basically copied from the existing get_cached_acl
function, but I'm a little uneasy with the above (and also with the
existing code that does the same thing).

The ba_refcount and ba_rcu are unioned. Once you've done the final
base_acl_put on the object, the you'll end up calling kfree_rcu which
is going to clobber the ba_refcount. How is it then safe to rely on it
still being zero for the atomic_inc_not_zero? ISTM that it would be
safer to make those separate fields and not union them.

...or am I missing something that prevents that?

> +void __forget_cached_acl(struct base_acl **p)
> +{
> +     struct base_acl *old;
> +
> +     old = xchg(p, ACL_NOT_CACHED);
> +     if (!is_uncached_acl(old))
> +             base_acl_put(old);
> +}
> +#endif
> +
> Âstatic void i_callback(struct rcu_head *head)
> Â{
> Â     struct inode *inode = container_of(head, struct inode, i_rcu);
> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
> index bc2693d..6c11909 100644
> --- a/fs/jffs2/acl.c
> +++ b/fs/jffs2/acl.c
> @@ -292,13 +292,15 @@ int jffs2_init_acl_post(struct inode *inode)
> Â     int rc;
> Â
> Â     if (inode->i_default_acl) {
> -             rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT, 
> inode->i_default_acl);
> +             rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT,
> +                             ÂÂÂÂÂposix_acl(inode->i_default_acl));
> Â             if (rc)
> Â                     return rc;
> Â     }
> Â
> Â     if (inode->i_acl) {
> -             rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS, 
> inode->i_acl);
> +             rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS,
> +                             ÂÂÂÂÂposix_acl(inode->i_acl));
> Â             if (rc)
> Â                     return rc;
> Â     }
> diff --git a/fs/namei.c b/fs/namei.c
> index 663933e..7a822d0 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -259,25 +259,28 @@ void putname(struct filename *name)
> Âstatic int check_acl(struct inode *inode, int mask)
> Â{
> Â#ifdef CONFIG_FS_POSIX_ACL
> -     struct posix_acl *acl;
> -
> Â     if (mask & MAY_NOT_BLOCK) {
> -             acl = get_cached_acl_rcu(inode, ACL_TYPE_ACCESS);
> -     ÂÂÂÂÂÂÂÂif (!acl)
> +             struct base_acl *base_acl;
> +
> +             base_acl = rcu_dereference(inode->i_acl);
> +     ÂÂÂÂÂÂÂÂif (!base_acl)
> Â     ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -EAGAIN;
> Â             /* no ->get_acl() calls in RCU mode... */
> -             if (is_uncached_acl(acl))
> +             if (is_uncached_acl(base_acl))
> Â                     return -ECHILD;
> -     ÂÂÂÂÂÂÂÂreturn posix_acl_permission(inode, acl, mask & ~MAY_NOT_BLOCK);
> -     }
> -
> -     acl = get_acl(inode, ACL_TYPE_ACCESS);
> -     if (IS_ERR(acl))
> -             return PTR_ERR(acl);
> -     if (acl) {
> -     ÂÂÂÂÂÂÂÂint error = posix_acl_permission(inode, acl, mask);
> -     ÂÂÂÂÂÂÂÂposix_acl_release(acl);
> -     ÂÂÂÂÂÂÂÂreturn error;
> +     ÂÂÂÂÂÂÂÂreturn posix_acl_permission(inode, posix_acl(base_acl),
> +                                     ÂÂÂÂmask & ~MAY_NOT_BLOCK);
> +     } else {
> +             struct posix_acl *acl;
> +
> +             acl = get_acl(inode, ACL_TYPE_ACCESS);
> +             if (IS_ERR(acl))
> +                     return PTR_ERR(acl);
> +             if (acl) {
> +                     int error = posix_acl_permission(inode, acl, mask);
> +                     posix_acl_release(acl);
> +                     return error;
> +             }
> Â     }
> Â#endif
> Â
> diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
> index 720d92f5..2b70944 100644
> --- a/fs/nfs/nfs3acl.c
> +++ b/fs/nfs/nfs3acl.c
> @@ -16,28 +16,28 @@
> Â * caching get_acl results in a race-free way.ÂÂSee fs/posix_acl.c:get_acl()
> Â * for explanations.
> Â */
> -static void nfs3_prepare_get_acl(struct posix_acl **p)
> +static void nfs3_prepare_get_acl(struct base_acl **p)
> Â{
> -     struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +     struct base_acl *sentinel = uncached_acl_sentinel(current);
> Â
> Â     if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) {
> Â             /* Not the first reader or sentinel already in place. */
> Â     }
> Â}
> Â
> -static void nfs3_complete_get_acl(struct posix_acl **p, struct posix_acl 
> *acl)
> +static void nfs3_complete_get_acl(struct base_acl **p, struct posix_acl *acl)
> Â{
> -     struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +     struct base_acl *sentinel = uncached_acl_sentinel(current);
> Â
> Â     /* Only cache the ACL if our sentinel is still in place. */
> Â     posix_acl_dup(acl);
> -     if (cmpxchg(p, sentinel, acl) != sentinel)
> +     if (cmpxchg(p, sentinel, &acl->a_base) != sentinel)
> Â             posix_acl_release(acl);
> Â}
> Â
> -static void nfs3_abort_get_acl(struct posix_acl **p)
> +static void nfs3_abort_get_acl(struct base_acl **p)
> Â{
> -     struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +     struct base_acl *sentinel = uncached_acl_sentinel(current);
> Â
> Â     /* Remove our sentinel upon failure. */
> Â     cmpxchg(p, sentinel, ACL_NOT_CACHED);
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index edc452c..1b685a1 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -21,7 +21,7 @@
> Â#include 
> Â#include 
> Â
> -static struct posix_acl **acl_by_type(struct inode *inode, int type)
> +static inline struct base_acl **acl_by_type(struct inode *inode, int type)
> Â{
> Â     switch (type) {
> Â     case ACL_TYPE_ACCESS:
> @@ -33,51 +33,23 @@ static struct posix_acl **acl_by_type(struct inode 
> *inode, int type)
> Â     }
> Â}
> Â
> -struct posix_acl *get_cached_acl(struct inode *inode, int type)
> +struct base_acl *get_cached_acl(struct inode *inode, int type)
> Â{
> -     struct posix_acl **p = acl_by_type(inode, type);
> -     struct posix_acl *acl;
> -
> -     for (;;) {
> -             rcu_read_lock();
> -             acl = rcu_dereference(*p);
> -             if (!acl || is_uncached_acl(acl) ||
> -             ÂÂÂÂatomic_inc_not_zero(&acl->a_refcount))
> -                     break;
> -             rcu_read_unlock();
> -             cpu_relax();
> -     }
> -     rcu_read_unlock();
> -     return acl;
> +     return __get_cached_acl(acl_by_type(inode, type));
> Â}
> ÂEXPORT_SYMBOL(get_cached_acl);
> Â
> -struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type)
> -{
> -     return rcu_dereference(*acl_by_type(inode, type));
> -}
> -EXPORT_SYMBOL(get_cached_acl_rcu);
> -
> Âvoid set_cached_acl(struct inode *inode, int type, struct posix_acl *acl)
> Â{
> -     struct posix_acl **p = acl_by_type(inode, type);
> -     struct posix_acl *old;
> +     struct base_acl **p = acl_by_type(inode, type);
> +     struct base_acl *old;
> Â
> -     old = xchg(p, posix_acl_dup(acl));
> +     old = xchg(p, &posix_acl_dup(acl)->a_base);
> Â     if (!is_uncached_acl(old))
> -             posix_acl_release(old);
> +             base_acl_put(old);
> Â}
> ÂEXPORT_SYMBOL(set_cached_acl);
> Â
> -static void __forget_cached_acl(struct posix_acl **p)
> -{
> -     struct posix_acl *old;
> -
> -     old = xchg(p, ACL_NOT_CACHED);
> -     if (!is_uncached_acl(old))
> -             posix_acl_release(old);
> -}
> -
> Âvoid forget_cached_acl(struct inode *inode, int type)
> Â{
> Â     __forget_cached_acl(acl_by_type(inode, type));
> @@ -93,25 +65,24 @@ EXPORT_SYMBOL(forget_all_cached_acls);
> Â
> Âstruct posix_acl *get_acl(struct inode *inode, int type)
> Â{
> -     void *sentinel;
> -     struct posix_acl **p;
> +     struct base_acl **p = acl_by_type(inode, type);
> +     struct base_acl *sentinel, *base_acl;
> Â     struct posix_acl *acl;
> Â
> +     if (!IS_POSIXACL(inode))
> +             return NULL;
> +
> Â     /*
> Â     Â* The sentinel is used to detect when another operation like
> Â     Â* set_cached_acl() or forget_cached_acl() races with get_acl().
> Â     Â* It is guaranteed that is_uncached_acl(sentinel) is true.
> Â     Â*/
> Â
> -     acl = get_cached_acl(inode, type);
> -     if (!is_uncached_acl(acl))
> -             return acl;
> -
> -     if (!IS_POSIXACL(inode))
> -             return NULL;
> +     base_acl = __get_cached_acl(p);
> +     if (!is_uncached_acl(base_acl))
> +             return posix_acl(base_acl);
> Â
> Â     sentinel = uncached_acl_sentinel(current);
> -     p = acl_by_type(inode, type);
> Â
> Â     /*
> Â     Â* If the ACL isn't being read yet, set our sentinel.ÂÂOtherwise, the
> @@ -151,7 +122,7 @@ struct posix_acl *get_acl(struct inode *inode, int type)
> Â     Â* Cache the result, but only if our sentinel is still in place.
> Â     Â*/
> Â     posix_acl_dup(acl);
> -     if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
> +     if (unlikely(cmpxchg(p, sentinel, &acl->a_base) != sentinel))
> Â             posix_acl_release(acl);
> Â     return acl;
> Â}
> @@ -163,7 +134,7 @@ EXPORT_SYMBOL(get_acl);
> Âvoid
> Âposix_acl_init(struct posix_acl *acl, int count)
> Â{
> -     atomic_set(&acl->a_refcount, 1);
> +     base_acl_init(&acl->a_base);
> Â     acl->a_count = count;
> Â}
> ÂEXPORT_SYMBOL(posix_acl_init);
> @@ -196,7 +167,7 @@ posix_acl_clone(const struct posix_acl *acl, gfp_t flags)
> Â             ÂÂÂÂÂÂÂÂÂÂÂsizeof(struct posix_acl_entry);
> Â             clone = kmemdup(acl, size, flags);
> Â             if (clone)
> -                     atomic_set(&clone->a_refcount, 1);
> +                     base_acl_init(&clone->a_base);
> Â     }
> Â     return clone;
> Â}
> @@ -418,7 +389,7 @@ static int posix_acl_create_masq(struct posix_acl *acl, 
> umode_t *mode_p)
> Â     umode_t mode = *mode_p;
> Â     int not_equiv = 0;
> Â
> -     /* assert(atomic_read(acl->a_refcount) == 1); */
> +     /* assert(base_acl_refcount(&acl->a_base) == 1); */
> Â
> Â     FOREACH_ACL_ENTRY(pa, acl, pe) {
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂswitch(pa->e_tag) {
> @@ -473,7 +444,7 @@ static int __posix_acl_chmod_masq(struct posix_acl *acl, 
> umode_t mode)
> Â     struct posix_acl_entry *group_obj = NULL, *mask_obj = NULL;
> Â     struct posix_acl_entry *pa, *pe;
> Â
> -     /* assert(atomic_read(acl->a_refcount) == 1); */
> +     /* assert(base_acl_refcount(&acl->a_base) == 1); */
> Â
> Â     FOREACH_ACL_ENTRY(pa, acl, pe) {
> Â             switch(pa->e_tag) {
> diff --git a/fs/richacl.c b/fs/richacl.c
> index cb0ef3f..8971ead 100644
> --- a/fs/richacl.c
> +++ b/fs/richacl.c
> @@ -31,7 +31,7 @@ richacl_alloc(int count, gfp_t gfp)
> Â     struct richacl *acl = kzalloc(size, gfp);
> Â
> Â     if (acl) {
> -             atomic_set(&acl->a_refcount, 1);
> +             base_acl_init(&acl->a_base);
> Â             acl->a_count = count;
> Â     }
> Â     return acl;
> @@ -50,7 +50,7 @@ richacl_clone(const struct richacl *acl, gfp_t gfp)
> Â
> Â     if (dup) {
> Â             memcpy(dup, acl, size);
> -             atomic_set(&dup->a_refcount, 1);
> +             base_acl_init(&dup->a_base);
> Â     }
> Â     return dup;
> Â}
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bb36561..06a30b0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -582,17 +582,23 @@ static inline void mapping_allow_writable(struct 
> address_space *mapping)
> Â#define i_size_ordered_init(inode) do { } while (0)
> Â#endif
> Â
> +struct base_acl {
> +     union {
> +             atomic_t ba_refcount;
> +             struct rcu_head ba_rcu;
> +     };
> +};
> Âstruct posix_acl;
> Â#define ACL_NOT_CACHED ((void *)(-1))
> Â
> -static inline struct posix_acl *
> +static inline struct base_acl *
> Âuncached_acl_sentinel(struct task_struct *task)
> Â{
> Â     return (void *)task + 1;
> Â}
> Â
> Âstatic inline bool
> -is_uncached_acl(struct posix_acl *acl)
> +is_uncached_acl(struct base_acl *acl)
> Â{
> Â     return (long)acl & 1;
> Â}
> @@ -613,9 +619,9 @@ struct inode {
> Â     kgid_t                  i_gid;
> Â     unsigned int            i_flags;
> Â
> -#ifdef CONFIG_FS_POSIX_ACL
> -     struct posix_acl        *i_acl;
> -     struct posix_acl        *i_default_acl;
> +#if defined(CONFIG_FS_POSIX_ACL)
> +     struct base_acl         *i_acl;
> +     struct base_acl         *i_default_acl;
> Â#endif
> Â
> Â     const struct inode_operations   *i_op;
> @@ -3193,4 +3199,29 @@ static inline bool dir_relax_shared(struct inode 
> *inode)
> Âextern bool path_noexec(const struct path *path);
> Âextern void inode_nohighmem(struct inode *inode);
> Â
> +static inline void base_acl_get(struct base_acl *acl)
> +{
> +     if (acl)
> +             atomic_inc(&acl->ba_refcount);
> +}
> +
> +static inline void base_acl_put(struct base_acl *acl)
> +{
> +     if (acl && atomic_dec_and_test(&acl->ba_refcount))
> +             kfree_rcu(acl, ba_rcu);
> +}
> +
> +static inline void base_acl_init(struct base_acl *acl)
> +{
> +     atomic_set(&acl->ba_refcount, 1);
> +}
> +
> +static inline int base_acl_refcount(struct base_acl *acl)
> +{
> +     return atomic_read(&acl->ba_refcount);
> +}
> +
> +extern struct base_acl *__get_cached_acl(struct base_acl **);
> +extern void __forget_cached_acl(struct base_acl **);
> +
> Â#endif /* _LINUX_FS_H */
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 5b5a80c..daf84fa 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -43,10 +43,7 @@ struct posix_acl_entry {
> Â};
> Â
> Âstruct posix_acl {
> -     union {
> -             atomic_t                a_refcount;
> -             struct rcu_head         a_rcu;
> -     };
> +     struct base_acl         a_base;ÂÂ/* must be first, see 
> posix_acl_release() */
> Â     unsigned int            a_count;
> Â     struct posix_acl_entry  a_entries[0];
> Â};
> @@ -61,8 +58,7 @@ struct posix_acl {
> Âstatic inline struct posix_acl *
> Âposix_acl_dup(struct posix_acl *acl)
> Â{
> -     if (acl)
> -             atomic_inc(&acl->a_refcount);
> +     base_acl_get(&acl->a_base);
> Â     return acl;
> Â}
> Â
> @@ -72,10 +68,16 @@ posix_acl_dup(struct posix_acl *acl)
> Âstatic inline void
> Âposix_acl_release(struct posix_acl *acl)
> Â{
> -     if (acl && atomic_dec_and_test(&acl->a_refcount))
> -             kfree_rcu(acl, a_rcu);
> +     BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
> +     base_acl_put(&acl->a_base);
> Â}
> Â
> +static inline struct posix_acl *
> +posix_acl(struct base_acl *base_acl)
> +{
> +     BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
> +     return container_of(base_acl, struct posix_acl, a_base);
> +}
> Â
> Â/* posix_acl.c */
> Â
> @@ -99,8 +101,7 @@ extern int posix_acl_create(struct inode *, umode_t *, 
> struct posix_acl **,
> Âextern int simple_set_acl(struct inode *, struct posix_acl *, int);
> Âextern int simple_acl_create(struct inode *, struct inode *);
> Â
> -struct posix_acl *get_cached_acl(struct inode *inode, int type);
> -struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type);
> +struct base_acl *get_cached_acl(struct inode *inode, int type);
> Âvoid set_cached_acl(struct inode *inode, int type, struct posix_acl *acl);
> Âvoid forget_cached_acl(struct inode *inode, int type);
> Âvoid forget_all_cached_acls(struct inode *inode);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index be9fb65..35a5bcb 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -31,7 +31,7 @@ struct richace {
> Â};
> Â
> Âstruct richacl {
> -     atomic_t        a_refcount;
> +     struct base_acl a_base;ÂÂ/* must be first, see richacl_put() */
> Â     unsigned int    a_owner_mask;
> Â     unsigned int    a_group_mask;
> Â     unsigned int    a_other_mask;
> @@ -56,8 +56,7 @@ struct richacl {
> Âstatic inline struct richacl *
> Ârichacl_get(struct richacl *acl)
> Â{
> -     if (acl)
> -             atomic_inc(&acl->a_refcount);
> +     base_acl_get(&acl->a_base);
> Â     return acl;
> Â}
> Â
> @@ -67,8 +66,8 @@ richacl_get(struct richacl *acl)
> Âstatic inline void
> Ârichacl_put(struct richacl *acl)
> Â{
> -     if (acl && atomic_dec_and_test(&acl->a_refcount))
> -             kfree(acl);
> +     BUILD_BUG_ON(offsetof(struct richacl, a_base) != 0);
> +     base_acl_put(&acl->a_base);
> Â}
> Â
> Â/**

--
Jeff Layton <jlayton@xxxxxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes, Jeff Layton <=