On Thu, 2017-11-30 at 22:12 +0800, Yan, Zheng wrote:
> negtive child dentry holds reference on inode's alias, it makes
> d_prune_aliases() do nothing.
>
> Signed-off-by: "Yan, Zheng" <zyan@xxxxxxxxxx>
> ---
> fs/ceph/mds_client.c | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index da181acd4a61..9e7bb5fa9295 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1440,6 +1440,30 @@ static int __close_session(struct ceph_mds_client *mdsc,
> return request_close_session(mdsc, session);
> }
>
> +static bool drop_negative_children(struct dentry *dentry)
> +{
> + struct dentry *child;
> + bool all_negtive = true;
> +
> + if (!d_is_dir(dentry))
> + goto out;
> +
> + spin_lock(&dentry->d_lock);
> + list_for_each_entry(child, &dentry->d_subdirs, d_child) {
> + if (d_really_is_positive(child)) {
> + all_negtive = false;
> + break;
> + }
> + }
> + spin_unlock(&dentry->d_lock);
> +
> + if (all_negtive)
> + shrink_dcache_parent(dentry);
> +out:
> + dput(dentry);
It's sort of nasty to drop the reference here. It makes it harder to
track how that gets done when you acquire and drop references across a
function boundary like this. There's also no reason for it since you
always do it last here anyway.
It'd be nicer to just move that into the caller.
> + return all_negtive;
> +}
> +
> /*
> * Trim old(er) caps.
> *
> @@ -1495,15 +1519,20 @@ static int trim_caps_cb(struct inode *inode, struct ceph_cap *cap, void *arg)
> __ceph_remove_cap(cap, true);
> session->s_trim_caps--;
> } else {
> - int refs;
> + struct dentry *dentry;
> /* try dropping referring dentries */
> spin_unlock(&ci->i_ceph_lock);
> - d_prune_aliases(inode);
> - refs = atomic_read(&inode->i_count);
> - if (refs == 1)
> - session->s_trim_caps--;
> - dout("trim_caps_cb %p cap %p pruned, count now %d\n",
> - inode, cap, refs);
> + dentry = d_find_any_alias(inode);
> + /* drop_negative_children() calls dput() */
> + if (dentry && drop_negative_children(dentry)) {
> + int refs;
> + d_prune_aliases(inode);
> + refs = atomic_read(&inode->i_count);
> + if (refs == 1)
> + session->s_trim_caps--;
> + dout("trim_caps_cb %p cap %p pruned, count now %d\n",
> + inode, cap, refs);
> + }
> return 0;
> }
>
I think the rest looks fine though.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
