Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190416174900.GT2217@ZenIV.linux.org.uk> <20190416175340.21068-1-viro@ZenIV.linux.org.uk>
 <20190416175340.21068-60-viro@ZenIV.linux.org.uk>
In-Reply-To: <20190416175340.21068-60-viro@ZenIV.linux.org.uk>
From:   Mike Marshall <hubcap@omnibond.com>
Date:   Mon, 22 Apr 2019 17:14:19 -0400
Message-ID: <CAOg9mSQppnRWPnSg9eAm0NeW3XEm0LZ74Aw651QTBx0Gm1jpQA@mail.gmail.com>
Subject: Re: [RFC PATCH 60/62] orangefs: make use of ->free_inode()
To:     Al Viro <viro@zeniv.linux.org.uk>,
        Martin Brandenburg <martin@omnibond.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Al...

I applied your "new inode method: ->free_inode()" and
"orangefs: make use of ->free_inode()" to our pagecache
branch (I hope to get it pulled in the next merge window).

I had to modify your "orangefs: make use of ->free_inode()" a
little, since Martin Brandenburg had already modified orangefs_i_callback
for the pagecache branch. I don't know for sure that my modifications
aren't nonsense :-) but I do know for sure that everything runs
with no xfstests regressions. I'll see what Martin thinks about
my changes...

orangefs_destroy_inode is pretty much a no-op now, so I guess
we'll get rid of it...

Acked-by: Mike Marshall <hubcap@omnibond.com>

Thanks...

-Mike


[root@vm1 linux]# git diff HEAD^ fs/orangefs/super.c
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 8fa30c13b7ed..f82ac9373443 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -125,20 +125,18 @@ static struct inode *orangefs_alloc_inode(struct
super_block *sb)
        return &orangefs_inode->vfs_inode;
 }

-static void orangefs_i_callback(struct rcu_head *head)
+static void orangefs_free_inode(struct inode *inode)
 {
-       struct inode *inode = container_of(head, struct inode, i_rcu);
-       struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
        struct orangefs_cached_xattr *cx;
        struct hlist_node *tmp;
        int i;

-       hash_for_each_safe(orangefs_inode->xattr_cache, i, tmp, cx, node) {
+       hash_for_each_safe(ORANGEFS_I(inode)->xattr_cache, i, tmp, cx, node) {
                hlist_del(&cx->node);
                kfree(cx);
        }

-       kmem_cache_free(orangefs_inode_cache, orangefs_inode);
+       kmem_cache_free(orangefs_inode_cache, ORANGEFS_I(inode));
 }

 static void orangefs_destroy_inode(struct inode *inode)
@@ -148,8 +146,6 @@ static void orangefs_destroy_inode(struct inode *inode)
        gossip_debug(GOSSIP_SUPER_DEBUG,
                        "%s: deallocated %p destroying inode %pU\n",
                        __func__, orangefs_inode, get_khandle_from_ino(inode));
-
-       call_rcu(&inode->i_rcu, orangefs_i_callback);
 }

 static int orangefs_write_inode(struct inode *inode,
@@ -316,6 +312,7 @@ void fsid_key_table_finalize(void)

 static const struct super_operations orangefs_s_ops = {
        .alloc_inode = orangefs_alloc_inode,
+       .free_inode = orangefs_free_inode,
        .destroy_inode = orangefs_destroy_inode,
        .write_inode = orangefs_write_inode,
        .drop_inode = generic_delete_inode,

On Tue, Apr 16, 2019 at 1:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/orangefs/super.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
> index dfaee90d30bd..3784f7e8b603 100644
> --- a/fs/orangefs/super.c
> +++ b/fs/orangefs/super.c
> @@ -124,11 +124,9 @@ static struct inode *orangefs_alloc_inode(struct super_block *sb)
>         return &orangefs_inode->vfs_inode;
>  }
>
> -static void orangefs_i_callback(struct rcu_head *head)
> +static void orangefs_free_inode(struct inode *inode)
>  {
> -       struct inode *inode = container_of(head, struct inode, i_rcu);
> -       struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
> -       kmem_cache_free(orangefs_inode_cache, orangefs_inode);
> +       kmem_cache_free(orangefs_inode_cache, ORANGEFS_I(inode));
>  }
>
>  static void orangefs_destroy_inode(struct inode *inode)
> @@ -138,8 +136,6 @@ static void orangefs_destroy_inode(struct inode *inode)
>         gossip_debug(GOSSIP_SUPER_DEBUG,
>                         "%s: deallocated %p destroying inode %pU\n",
>                         __func__, orangefs_inode, get_khandle_from_ino(inode));
> -
> -       call_rcu(&inode->i_rcu, orangefs_i_callback);
>  }
>
>  /*
> @@ -299,6 +295,7 @@ void fsid_key_table_finalize(void)
>
>  static const struct super_operations orangefs_s_ops = {
>         .alloc_inode = orangefs_alloc_inode,
> +       .free_inode = orangefs_free_inode,
>         .destroy_inode = orangefs_destroy_inode,
>         .drop_inode = generic_delete_inode,
>         .statfs = orangefs_statfs,
> --
> 2.11.0
>