linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vfs: call d_op->d_prune() before unhashing dentry
@ 2013-08-13  7:42 Yan, Zheng
  2013-08-22  5:10 ` Sage Weil
  0 siblings, 1 reply; 3+ messages in thread
From: Yan, Zheng @ 2013-08-13  7:42 UTC (permalink / raw)
  To: viro, sage; +Cc: linux-fsdevel, Yan, Zheng

From: "Yan, Zheng" <zheng.z.yan@intel.com>

The d_prune dentry operation is used to notify filesystem when VFS
about to prune a hashed dentry from the dcache. There are three
code paths that prune dentries: shrink_dcache_for_umount_subtree(),
prune_dcache_sb() and d_prune_aliases(). For the d_prune_aliases()
case, VFS unhashes the dentry first, then call the d_prune dentry
operation. This confuses ceph_d_prune() (ceph uses the d_prune
dentry operation to maintain a flag indicating whether the complete
contents of a directory are in the dcache, pruning unhashed dentry
does not affect dir's completeness)

This patch fixes the issue by calling the d_prune dentry operation
in d_prune_aliases(), before unhashing the dentry. Also make VFS
only call the d_prune dentry operation for hashed dentry, to avoid
calling the d_prune dentry operation twice when dentry is pruned
by d_prune_aliases().

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 fs/dcache.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 87bdb53..612e99b 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -472,7 +472,7 @@ relock:
 	 * inform the fs via d_prune that this dentry is about to be
 	 * unhashed and destroyed.
 	 */
-	if (dentry->d_flags & DCACHE_OP_PRUNE)
+	if ((dentry->d_flags & DCACHE_OP_PRUNE) && !d_unhashed(dentry))
 		dentry->d_op->d_prune(dentry);
 
 	dentry_lru_del(dentry);
@@ -719,6 +719,14 @@ restart:
 	hlist_for_each_entry(dentry, &inode->i_dentry, d_alias) {
 		spin_lock(&dentry->d_lock);
 		if (!dentry->d_count) {
+			/*
+			 * inform the fs via d_prune that this dentry
+			 * is about to be unhashed and destroyed.
+			 */
+			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
+			    !d_unhashed(dentry))
+				dentry->d_op->d_prune(dentry);
+
 			__dget_dlock(dentry);
 			__d_drop(dentry);
 			spin_unlock(&dentry->d_lock);
@@ -907,7 +915,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
 			 * inform the fs that this dentry is about to be
 			 * unhashed and destroyed.
 			 */
-			if (dentry->d_flags & DCACHE_OP_PRUNE)
+			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
+			    !d_unhashed(dentry))
 				dentry->d_op->d_prune(dentry);
 
 			dentry_lru_del(dentry);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] vfs: call d_op->d_prune() before unhashing dentry
  2013-08-13  7:42 [PATCH] vfs: call d_op->d_prune() before unhashing dentry Yan, Zheng
@ 2013-08-22  5:10 ` Sage Weil
  2013-09-03 15:58   ` Sage Weil
  0 siblings, 1 reply; 3+ messages in thread
From: Sage Weil @ 2013-08-22  5:10 UTC (permalink / raw)
  To: viro, Yan, Zheng; +Cc: linux-fsdevel

On Tue, 13 Aug 2013, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> The d_prune dentry operation is used to notify filesystem when VFS
> about to prune a hashed dentry from the dcache. There are three
> code paths that prune dentries: shrink_dcache_for_umount_subtree(),
> prune_dcache_sb() and d_prune_aliases(). For the d_prune_aliases()
> case, VFS unhashes the dentry first, then call the d_prune dentry
> operation. This confuses ceph_d_prune() (ceph uses the d_prune
> dentry operation to maintain a flag indicating whether the complete
> contents of a directory are in the dcache, pruning unhashed dentry
> does not affect dir's completeness)
> 
> This patch fixes the issue by calling the d_prune dentry operation
> in d_prune_aliases(), before unhashing the dentry. Also make VFS
> only call the d_prune dentry operation for hashed dentry, to avoid
> calling the d_prune dentry operation twice when dentry is pruned
> by d_prune_aliases().
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>

Reviewed-by: Sage Weil <sage@inktank.com>

This is behaving well under my testing, seems to resolve the race we were 
encoutering before, and cleans up the d_prune semantics nicely IMO.

Al, does this look okay to you?

Thanks!
sage

> ---
>  fs/dcache.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 87bdb53..612e99b 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -472,7 +472,7 @@ relock:
>  	 * inform the fs via d_prune that this dentry is about to be
>  	 * unhashed and destroyed.
>  	 */
> -	if (dentry->d_flags & DCACHE_OP_PRUNE)
> +	if ((dentry->d_flags & DCACHE_OP_PRUNE) && !d_unhashed(dentry))
>  		dentry->d_op->d_prune(dentry);
>  
>  	dentry_lru_del(dentry);
> @@ -719,6 +719,14 @@ restart:
>  	hlist_for_each_entry(dentry, &inode->i_dentry, d_alias) {
>  		spin_lock(&dentry->d_lock);
>  		if (!dentry->d_count) {
> +			/*
> +			 * inform the fs via d_prune that this dentry
> +			 * is about to be unhashed and destroyed.
> +			 */
> +			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
> +			    !d_unhashed(dentry))
> +				dentry->d_op->d_prune(dentry);
> +
>  			__dget_dlock(dentry);
>  			__d_drop(dentry);
>  			spin_unlock(&dentry->d_lock);
> @@ -907,7 +915,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
>  			 * inform the fs that this dentry is about to be
>  			 * unhashed and destroyed.
>  			 */
> -			if (dentry->d_flags & DCACHE_OP_PRUNE)
> +			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
> +			    !d_unhashed(dentry))
>  				dentry->d_op->d_prune(dentry);
>  
>  			dentry_lru_del(dentry);
> -- 
> 1.8.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vfs: call d_op->d_prune() before unhashing dentry
  2013-08-22  5:10 ` Sage Weil
@ 2013-09-03 15:58   ` Sage Weil
  0 siblings, 0 replies; 3+ messages in thread
From: Sage Weil @ 2013-09-03 15:58 UTC (permalink / raw)
  To: viro; +Cc: Yan, Zheng, linux-fsdevel, hch

Hi Al,

We'd like to get this into mainline during this window.  Does this patch 
look good to you?

Thanks-
sage


On Wed, 21 Aug 2013, Sage Weil wrote:

> On Tue, 13 Aug 2013, Yan, Zheng wrote:
> > From: "Yan, Zheng" <zheng.z.yan@intel.com>
> > 
> > The d_prune dentry operation is used to notify filesystem when VFS
> > about to prune a hashed dentry from the dcache. There are three
> > code paths that prune dentries: shrink_dcache_for_umount_subtree(),
> > prune_dcache_sb() and d_prune_aliases(). For the d_prune_aliases()
> > case, VFS unhashes the dentry first, then call the d_prune dentry
> > operation. This confuses ceph_d_prune() (ceph uses the d_prune
> > dentry operation to maintain a flag indicating whether the complete
> > contents of a directory are in the dcache, pruning unhashed dentry
> > does not affect dir's completeness)
> > 
> > This patch fixes the issue by calling the d_prune dentry operation
> > in d_prune_aliases(), before unhashing the dentry. Also make VFS
> > only call the d_prune dentry operation for hashed dentry, to avoid
> > calling the d_prune dentry operation twice when dentry is pruned
> > by d_prune_aliases().
> > 
> > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> 
> Reviewed-by: Sage Weil <sage@inktank.com>
> 
> This is behaving well under my testing, seems to resolve the race we were 
> encoutering before, and cleans up the d_prune semantics nicely IMO.
> 
> Al, does this look okay to you?
> 
> Thanks!
> sage
> 
> > ---
> >  fs/dcache.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index 87bdb53..612e99b 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -472,7 +472,7 @@ relock:
> >  	 * inform the fs via d_prune that this dentry is about to be
> >  	 * unhashed and destroyed.
> >  	 */
> > -	if (dentry->d_flags & DCACHE_OP_PRUNE)
> > +	if ((dentry->d_flags & DCACHE_OP_PRUNE) && !d_unhashed(dentry))
> >  		dentry->d_op->d_prune(dentry);
> >  
> >  	dentry_lru_del(dentry);
> > @@ -719,6 +719,14 @@ restart:
> >  	hlist_for_each_entry(dentry, &inode->i_dentry, d_alias) {
> >  		spin_lock(&dentry->d_lock);
> >  		if (!dentry->d_count) {
> > +			/*
> > +			 * inform the fs via d_prune that this dentry
> > +			 * is about to be unhashed and destroyed.
> > +			 */
> > +			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
> > +			    !d_unhashed(dentry))
> > +				dentry->d_op->d_prune(dentry);
> > +
> >  			__dget_dlock(dentry);
> >  			__d_drop(dentry);
> >  			spin_unlock(&dentry->d_lock);
> > @@ -907,7 +915,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
> >  			 * inform the fs that this dentry is about to be
> >  			 * unhashed and destroyed.
> >  			 */
> > -			if (dentry->d_flags & DCACHE_OP_PRUNE)
> > +			if ((dentry->d_flags & DCACHE_OP_PRUNE) &&
> > +			    !d_unhashed(dentry))
> >  				dentry->d_op->d_prune(dentry);
> >  
> >  			dentry_lru_del(dentry);
> > -- 
> > 1.8.1.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-09-03 15:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-13  7:42 [PATCH] vfs: call d_op->d_prune() before unhashing dentry Yan, Zheng
2013-08-22  5:10 ` Sage Weil
2013-09-03 15:58   ` Sage Weil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).