From: Miklos Szeredi <[email protected]>
The d_instantiate(new_dentry, NULL) is superfluous, the dentry is
already negative. Rehashing this dummy dentry isn't needed either,
d_move() works fine on an unhashed target.
The re-checking for busy after a failed nfs_sillyrename() is bogus
too: new_dentry->d_count < 2 would be a bug here.
Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/nfs/dir.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Index: linux-2.6/fs/nfs/dir.c
===================================================================
--- linux-2.6.orig/fs/nfs/dir.c 2009-12-02 15:11:19.000000000 +0100
+++ linux-2.6/fs/nfs/dir.c 2009-12-02 15:11:30.000000000 +0100
@@ -1611,14 +1611,11 @@ static int nfs_rename(struct inode *old_
/* silly-rename the existing target ... */
err = nfs_sillyrename(new_dir, new_dentry);
- if (!err) {
- new_dentry = rehash = dentry;
- new_inode = NULL;
- /* instantiate the replacement target */
- d_instantiate(new_dentry, NULL);
- } else if (atomic_read(&new_dentry->d_count) > 1)
- /* dentry still busy? */
+ if (err)
goto out;
+
+ new_dentry = dentry;
+ new_inode = NULL;
}
}
--
Miklos,
Don't we need to make the 'dentry' hashed, in sillyrename case?
The d_alloc()ed dentry is unhashed, and it was d_rehash()ed later
(before the patch).
new_dentry = rehash = dentry;
:::
out:
if (rehash)
d_rehash(rehash);
Applying your patch, the 'rehash' is not re-assigned and the 'dentry'
left unhashed. Such dentry which is visible by readdir but its d_hash is
empty, will be a problem in nfs readdir (which calls d_lookup()), I am
afraid.
J. R. Okajima
Miklos Szeredi:
> The d_instantiate(new_dentry, NULL) is superfluous, the dentry is
> already negative. Rehashing this dummy dentry isn't needed either,
> d_move() works fine on an unhashed target.
>
> The re-checking for busy after a failed nfs_sillyrename() is bogus
> too: new_dentry->d_count < 2 would be a bug here.
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> ---
> fs/nfs/dir.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> Index: linux-2.6/fs/nfs/dir.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/dir.c 2009-12-02 15:11:19.000000000 +0100
> +++ linux-2.6/fs/nfs/dir.c 2009-12-02 15:11:30.000000000 +0100
> @@ -1611,14 +1611,11 @@ static int nfs_rename(struct inode *old_
>
> /* silly-rename the existing target ... */
> err = nfs_sillyrename(new_dir, new_dentry);
> - if (!err) {
> - new_dentry = rehash = dentry;
> - new_inode = NULL;
> - /* instantiate the replacement target */
> - d_instantiate(new_dentry, NULL);
> - } else if (atomic_read(&new_dentry->d_count) > 1)
> - /* dentry still busy? */
> + if (err)
> goto out;
> +
> + new_dentry = dentry;
> + new_inode = NULL;
> }
> }
Miklos Szeredi <[email protected]> writes:
> --- linux-2.6.orig/fs/nfs/dir.c 2009-12-02 15:11:19.000000000 +0100
> +++ linux-2.6/fs/nfs/dir.c 2009-12-02 15:11:30.000000000 +0100
> @@ -1611,14 +1611,11 @@ static int nfs_rename(struct inode *old_
>
> /* silly-rename the existing target ... */
> err = nfs_sillyrename(new_dir, new_dentry);
> - if (!err) {
> - new_dentry = rehash = dentry;
> - new_inode = NULL;
> - /* instantiate the replacement target */
> - d_instantiate(new_dentry, NULL);
> - } else if (atomic_read(&new_dentry->d_count) > 1)
> - /* dentry still busy? */
> + if (err)
> goto out;
> +
> + new_dentry = dentry;
> + new_inode = NULL;
This needs to update "rehash". Sorry, this patch is still compile test
only, although the patch is clear and simple.
Thanks.
--
OGAWA Hirofumi <[email protected]>
[PATCH] nfs: Fix d_rehash() for hashed dentry in nfs_rename()
Recent change is missing to update "rehash". With that change, it will
become the cause of adding dentry to hash twice.
This explains the reason of Oops (dereference the freed dentry in
__d_lookup()) on my machine.
Signed-off-by: OGAWA Hirofumi <[email protected]>
---
fs/nfs/dir.c | 1 +
1 file changed, 1 insertion(+)
diff -puN fs/nfs/dir.c~nfs-d_rehash-fix fs/nfs/dir.c
--- linux-2.6/fs/nfs/dir.c~nfs-d_rehash-fix 2009-12-28 06:18:09.000000000 +0900
+++ linux-2.6-hirofumi/fs/nfs/dir.c 2009-12-28 06:18:16.000000000 +0900
@@ -1615,6 +1615,7 @@ static int nfs_rename(struct inode *old_
goto out;
new_dentry = dentry;
+ rehash = NULL;
new_inode = NULL;
}
}
_