[PATCH 13/19] ext4: remove unnecessary dentry_unhash on rmdir/rename_dir

[PATCH 13/19] ext4: remove unnecessary dentry_unhash on rmdir/rename_dir

[Sage Weil]

ext4 has no problems with lingering references to unlinked directory
inodes.

CC: "Theodore Ts'o" <tytso@mit.edu>
CC: Andreas Dilger <adilger.kernel@dilger.ca>
CC: linux-ext4@vger.kernel.org
Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/ext4/namei.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 792d06e..67fd0b0 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2123,8 +2123,6 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
 	struct ext4_dir_entry_2 *de;
 	handle_t *handle;
 
-	dentry_unhash(dentry);
-
 	/* Initialize quotas before so that eventual writes go in
 	 * separate transaction */
 	dquot_initialize(dir);
@@ -2352,9 +2350,6 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct ext4_dir_entry_2 *old_de, *new_de;
 	int retval, force_da_alloc = 0;
 
-	if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
-		dentry_unhash(new_dentry);
-
 	dquot_initialize(old_dir);
 	dquot_initialize(new_dir);
 
-- 
1.7.0

[PATCH 14/19] ext3: remove unnecessary dentry_unhash on rmdir/rename_dir

[Sage Weil]

ext3 has no problems with lingering references to unlinked directory
inodes.

CC: Jan Kara <jack@suse.cz>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Andreas Dilger <adilger.kernel@dilger.ca>
CC: linux-ext4@vger.kernel.org
Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/ext3/namei.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index f89b1d4..32f3b86 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -2074,8 +2074,6 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
 	struct ext3_dir_entry_2 * de;
 	handle_t *handle;
 
-	dentry_unhash(dentry);
-
 	/* Initialize quotas before so that eventual writes go in
 	 * separate transaction */
 	dquot_initialize(dir);
@@ -2298,9 +2296,6 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
 	struct ext3_dir_entry_2 * old_de, * new_de;
 	int retval, flush_file = 0;
 
-	if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
-		dentry_unhash(new_dentry);
-
 	dquot_initialize(old_dir);
 	dquot_initialize(new_dir);
 
-- 
1.7.0

[PATCH 15/19] ext2: remove unnecessary dentry_unhash on rmdir/rename_dir

[Sage Weil]

ext2 has no problems with lingering references to unlinked directory
inodes.

CC: Jan Kara <jack@suse.cz>
CC: linux-ext4@vger.kernel.org
Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/ext2/namei.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index 516c31d..ed5c5d4 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -296,8 +296,6 @@ static int ext2_rmdir (struct inode * dir, struct dentry *dentry)
 	struct inode * inode = dentry->d_inode;
 	int err = -ENOTEMPTY;
 
-	dentry_unhash(dentry);
-
 	if (ext2_empty_dir(inode)) {
 		err = ext2_unlink(dir, dentry);
 		if (!err) {
@@ -320,9 +318,6 @@ static int ext2_rename (struct inode * old_dir, struct dentry * old_dentry,
 	struct ext2_dir_entry_2 * old_de;
 	int err = -ENOENT;
 
-	if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
-		dentry_unhash(new_dentry);
-
 	dquot_initialize(old_dir);
 	dquot_initialize(new_dir);
 
-- 
1.7.0

Re: [PATCH 14/19] ext3: remove unnecessary dentry_unhash on rmdir/rename_dir

[Jan Kara]

On Tue 24-05-11 13:06:17, Sage Weil wrote:
> ext3 has no problems with lingering references to unlinked directory
> inodes.
  OK, so if I understand right, dentry_unhash() has been there only so that
filesystem can detect whether (something under) removed directory is in
use? So filesystems which can happily handle unlinked but open directories
don't need it, right? If that's the case, you can add:
  Acked-by: Jan Kara <jack@suse.cz>
to this patch and also the ext2 version.

								Honza
> 
> CC: Jan Kara <jack@suse.cz>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Andreas Dilger <adilger.kernel@dilger.ca>
> CC: linux-ext4@vger.kernel.org
> Signed-off-by: Sage Weil <sage@newdream.net>
> ---
>  fs/ext3/namei.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index f89b1d4..32f3b86 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -2074,8 +2074,6 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
>  	struct ext3_dir_entry_2 * de;
>  	handle_t *handle;
>  
> -	dentry_unhash(dentry);
> -
>  	/* Initialize quotas before so that eventual writes go in
>  	 * separate transaction */
>  	dquot_initialize(dir);
> @@ -2298,9 +2296,6 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
>  	struct ext3_dir_entry_2 * old_de, * new_de;
>  	int retval, flush_file = 0;
>  
> -	if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
> -		dentry_unhash(new_dentry);
> -
>  	dquot_initialize(old_dir);
>  	dquot_initialize(new_dir);
>  
> -- 
> 1.7.0
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 14/19] ext3: remove unnecessary dentry_unhash on rmdir/rename_dir

[Sage Weil]

On Tue, 24 May 2011, Jan Kara wrote:
> On Tue 24-05-11 13:06:17, Sage Weil wrote:
> > ext3 has no problems with lingering references to unlinked directory
> > inodes.
>   OK, so if I understand right, dentry_unhash() has been there only so that
> filesystem can detect whether (something under) removed directory is in
> use? So filesystems which can happily handle unlinked but open directories
> don't need it, right? If that's the case, you can add:
>   Acked-by: Jan Kara <jack@suse.cz>
> to this patch and also the ext2 version.

Right.  Basically, a simple fs can return EBUSY if the dentry is hashed 
(implying there are still references) and not worry about a racing process 
traversing into the directory while rmdir is running.  A sane fs can 
handle references and doesn't care if a racing process traverses into the 
dir before the ->rmdir method completes.

Thanks!
sage

Re: [PATCH 14/19] ext3: remove unnecessary dentry_unhash on rmdir/rename_dir

[Yongqiang Yang]

Hi,

Which kernel version is this patch based on?
Code in my working tree which is 2.6.39-rc3 is already same as the
code after the patch applied.

Thx!

Yongqiang.
On Wed, May 25, 2011 at 4:06 AM, Sage Weil <sage@newdream.net> wrote:
> ext3 has no problems with lingering references to unlinked directory
> inodes.
>
> CC: Jan Kara <jack@suse.cz>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Andreas Dilger <adilger.kernel@dilger.ca>
> CC: linux-ext4@vger.kernel.org
> Signed-off-by: Sage Weil <sage@newdream.net>
> ---
>  fs/ext3/namei.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index f89b1d4..32f3b86 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -2074,8 +2074,6 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
>        struct ext3_dir_entry_2 * de;
>        handle_t *handle;
>
> -       dentry_unhash(dentry);
> -
>        /* Initialize quotas before so that eventual writes go in
>         * separate transaction */
>        dquot_initialize(dir);
> @@ -2298,9 +2296,6 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
>        struct ext3_dir_entry_2 * old_de, * new_de;
>        int retval, flush_file = 0;
>
> -       if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
> -               dentry_unhash(new_dentry);
> -
>        dquot_initialize(old_dir);
>        dquot_initialize(new_dir);
>
> --
> 1.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/19] ext3: remove unnecessary dentry_unhash on rmdir/rename_dir

[Sage Weil]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2300 bytes --]

On Wed, 25 May 2011, Yongqiang Yang wrote:
> Hi,
> 
> Which kernel version is this patch based on?
> Code in my working tree which is 2.6.39-rc3 is already same as the
> code after the patch applied.

An earlier patch in the series pushes the dentry_unhash call in fs/namei.c 
down into each file system (on change in behavior).  This patch then
removes the call because it shouldn't be necessary for extN, or any other 
file system that doesn't have problems with racing processes getting
references to the just-removed directory inode.

sage


> 
> Thx!
> 
> Yongqiang.
> On Wed, May 25, 2011 at 4:06 AM, Sage Weil <sage@newdream.net> wrote:
> > ext3 has no problems with lingering references to unlinked directory
> > inodes.
> >
> > CC: Jan Kara <jack@suse.cz>
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > CC: linux-ext4@vger.kernel.org
> > Signed-off-by: Sage Weil <sage@newdream.net>
> > ---
> >  fs/ext3/namei.c |    5 -----
> >  1 files changed, 0 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> > index f89b1d4..32f3b86 100644
> > --- a/fs/ext3/namei.c
> > +++ b/fs/ext3/namei.c
> > @@ -2074,8 +2074,6 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
> >        struct ext3_dir_entry_2 * de;
> >        handle_t *handle;
> >
> > -       dentry_unhash(dentry);
> > -
> >        /* Initialize quotas before so that eventual writes go in
> >         * separate transaction */
> >        dquot_initialize(dir);
> > @@ -2298,9 +2296,6 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
> >        struct ext3_dir_entry_2 * old_de, * new_de;
> >        int retval, flush_file = 0;
> >
> > -       if (new_dentry->d_inode && S_ISDIR(new_dentry->d_inode->i_mode))
> > -               dentry_unhash(new_dentry);
> > -
> >        dquot_initialize(old_dir);
> >        dquot_initialize(new_dir);
> >
> > --
> > 1.7.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 
> 
> -- 
> Best Wishes
> Yongqiang Yang
> 
>