2011-05-24 20:06:17

by Sage Weil

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

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

CC: Jan Kara <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Andreas Dilger <[email protected]>
CC: [email protected]
Signed-off-by: Sage Weil <[email protected]>
---
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



2011-05-24 20:38:07

by Jan Kara

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

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 <[email protected]>
to this patch and also the ext2 version.

Honza
>
> CC: Jan Kara <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Andreas Dilger <[email protected]>
> CC: [email protected]
> Signed-off-by: Sage Weil <[email protected]>
> ---
> 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 <[email protected]>
SUSE Labs, CR

2011-05-24 23:57:53

by Sage Weil

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

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 <[email protected]>
> 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

2011-05-25 02:44:31

by Yongqiang Yang

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

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 <[email protected]> wrote:
> ext3 has no problems with lingering references to unlinked directory
> inodes.
>
> CC: Jan Kara <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Andreas Dilger <[email protected]>
> CC: [email protected]
> Signed-off-by: Sage Weil <[email protected]>
> ---
> ?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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
Best Wishes
Yongqiang Yang

2011-05-25 03:44:49

by Sage Weil

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

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 <[email protected]> wrote:
> > ext3 has no problems with lingering references to unlinked directory
> > inodes.
> >
> > CC: Jan Kara <[email protected]>
> > CC: Andrew Morton <[email protected]>
> > CC: Andreas Dilger <[email protected]>
> > CC: [email protected]
> > Signed-off-by: Sage Weil <[email protected]>
> > ---
> > ?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 [email protected]
> > More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> >
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>
>