2007-11-10 23:03:56

by Kumar Gala

[permalink] [raw]
Subject: [PATCH] NFS: Fix the ustat() regression

From: Trond Myklebust <[email protected]>

Since 2.6.18, the superblock sb->s_root has been a dummy dentry with a
dummy inode. This breaks ustat(), which actually uses sb->s_root in a
vfstat() call.

Fix this by making the s_root a dummy alias to the directory inode that was
used when creating the superblock.

Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Kumar Gala <[email protected]>
---
fs/nfs/getroot.c | 81 ++++++++++++++++++------------------------------------
1 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index 522e5ad..0ee4384 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -43,6 +43,25 @@
#define NFSDBG_FACILITY NFSDBG_CLIENT

/*
+ * Set the superblock root dentry.
+ * Note that this function frees the inode in case of error.
+ */
+static int nfs_superblock_set_dummy_root(struct super_block *sb, struct inode *inode)
+{
+ /* The mntroot acts as the dummy root dentry for this superblock */
+ if (sb->s_root == NULL) {
+ sb->s_root = d_alloc_root(inode);
+ if (sb->s_root == NULL) {
+ iput(inode);
+ return -ENOMEM;
+ }
+ /* Circumvent igrab(): we know the inode is not being freed */
+ atomic_inc(&inode->i_count);
+ }
+ return 0;
+}
+
+/*
* get an NFS2/NFS3 root dentry from the root filehandle
*/
struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh)
@@ -54,33 +73,6 @@ struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh)
struct inode *inode;
int error;

- /* create a dummy root dentry with dummy inode for this superblock */
- if (!sb->s_root) {
- struct nfs_fh dummyfh;
- struct dentry *root;
- struct inode *iroot;
-
- memset(&dummyfh, 0, sizeof(dummyfh));
- memset(&fattr, 0, sizeof(fattr));
- nfs_fattr_init(&fattr);
- fattr.valid = NFS_ATTR_FATTR;
- fattr.type = NFDIR;
- fattr.mode = S_IFDIR | S_IRUSR | S_IWUSR;
- fattr.nlink = 2;
-
- iroot = nfs_fhget(sb, &dummyfh, &fattr);
- if (IS_ERR(iroot))
- return ERR_PTR(PTR_ERR(iroot));
-
- root = d_alloc_root(iroot);
- if (!root) {
- iput(iroot);
- return ERR_PTR(-ENOMEM);
- }
-
- sb->s_root = root;
- }
-
/* get the actual root for this mount */
fsinfo.fattr = &fattr;

@@ -96,6 +88,10 @@ struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh)
return ERR_PTR(PTR_ERR(inode));
}

+ error = nfs_superblock_set_dummy_root(sb, inode);
+ if (error != 0)
+ return ERR_PTR(error);
+
/* root dentries normally start off anonymous and get spliced in later
* if the dentry tree reaches them; however if the dentry already
* exists, we'll pick it up at this point and use it as the root
@@ -241,33 +237,6 @@ struct dentry *nfs4_get_root(struct super_block *sb, struct nfs_fh *mntfh)

dprintk("--> nfs4_get_root()\n");

- /* create a dummy root dentry with dummy inode for this superblock */
- if (!sb->s_root) {
- struct nfs_fh dummyfh;
- struct dentry *root;
- struct inode *iroot;
-
- memset(&dummyfh, 0, sizeof(dummyfh));
- memset(&fattr, 0, sizeof(fattr));
- nfs_fattr_init(&fattr);
- fattr.valid = NFS_ATTR_FATTR;
- fattr.type = NFDIR;
- fattr.mode = S_IFDIR | S_IRUSR | S_IWUSR;
- fattr.nlink = 2;
-
- iroot = nfs_fhget(sb, &dummyfh, &fattr);
- if (IS_ERR(iroot))
- return ERR_PTR(PTR_ERR(iroot));
-
- root = d_alloc_root(iroot);
- if (!root) {
- iput(iroot);
- return ERR_PTR(-ENOMEM);
- }
-
- sb->s_root = root;
- }
-
/* get the info about the server and filesystem */
error = nfs4_server_capabilities(server, mntfh);
if (error < 0) {
@@ -289,6 +258,10 @@ struct dentry *nfs4_get_root(struct super_block *sb, struct nfs_fh *mntfh)
return ERR_PTR(PTR_ERR(inode));
}

+ error = nfs_superblock_set_dummy_root(sb, inode);
+ if (error != 0)
+ return ERR_PTR(error);
+
/* root dentries normally start off anonymous and get spliced in later
* if the dentry tree reaches them; however if the dentry already
* exists, we'll pick it up at this point and use it as the root
--
1.5.3.4


2007-11-12 22:23:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix the ustat() regression

On Sat, 10 Nov 2007 17:02:19 -0600 (CST) Kumar Gala <[email protected]> wrote:

> Since 2.6.18, the superblock sb->s_root has been a dummy dentry with a
> dummy inode. This breaks ustat(), which actually uses sb->s_root in a
> vfstat() call.
>
> Fix this by making the s_root a dummy alias to the directory inode that was
> used when creating the superblock.

I already have this in my
things-which-need-to-go-into-2.6.24-via-maintainers queue.


That queue is huge:

i-oat-add-support-for-version-2-of-ioatdma-device.patch
reiserfs-dont-drop-pg_dirty-when-releasing-sub-page-sized-dirty-file.patch
rtc-release-correct-region-in-error-path.patch
rtc-fallback-to-requesting-only-the-ports-we-actually-use.patch
i5000_edac-no-need-to-__stringify-kbuild_basename.patch
serial-only-use-pnp-irq-if-its-valid.patch
sunrpc-xprtrdma-transportc-fix-use-after-free.patch
fix-mm-utilckrealloc.patch
fuse_file_alloc-fix-null-dereferences.patch
tle62x0-driver-stops-ignoring-read-errors.patch
rd-fix-data-corruption-on-memory-pressure.patch
cris-gpio-undo-locks-before-returning.patch
mips-undo-locking-on-error-path-returns.patch
mips-undo-locking-on-error-path-returns-checkpatch-fixes.patch
nfs-fix-the-ustat-regression.patch
proc-simplify-and-correct-proc_flush_task.patch
fix-param_sysfs_builtin-name-length-check.patch
rtc-convert-mutex-to-bitfield.patch
mark-sys_open-sys_read-exports-unused.patch
sysctl-fix-token-ring-procname.patch
gbefb-fix-section-mismatch-warnings.patch
vmstat-fix-section-mismatch-warning.patch
pidns-place-under-config_experimental.patch
pidns-place-under-config_experimental-checkpatch-fixes.patch
__do_irq-does-not-check-irq_disabled-when-irq_per_cpu-is-set.patch
hibernate-fix-lockdep-report-2.patch
smbfs-fix-debug-builds.patch
fix-64kb-blocksize-in-ext3-directories.patch
fix-64kb-blocksize-in-ext3-directories-checkpatch-fixes.patch
uml-fix-spurious-irq-testing.patch
uml-remove-last-include-of-libc-asm-pageh.patch
uml-fix-build-for-config_tcp.patch
uml-fix-build-for-config_printk.patch
markers-fix-warnings.patch
swap-delay-accounting-include-lock_page-delays.patch
file-capabilities-allow-sigcont-within-session-v2.patch
file-capabilities-allow-sigcont-within-session-v2-checkpatch-fixes.patch
file-capabilities-allow-sigcont-within-session-v2-file-capabilities-remove-the-non-matching-uid-special-case-for-kill.patch
feature-removal-schedule-remove-sa_-flags-entry.patch
kernel-taskstatsc-fix-bogus-nlmsg_free.patch
x86-show-cpuinfo-only-for-online-cpus.patch
make-proc-acpi-ac_adapter-dependent-on-acpi_procfs.patch
acpi-ac-update-ac-state-on-resume.patch
keyspan-init-termios-properly.patch
x86-disable-preemption-in-delay_tsc.patch
tty-fix-network-driver-interactions-with-tcget-set-checkpatch-fixes.patch
tty-fix-tty-network-driver-interactions-with-tcget-tcset-calls-x86-fix.patch
x86_64-fix-cpu-hotplug-regression.patch
oprofile-fix-oops-on-x86-32-bit.patch
x86-32-bit-ptrace-emulation-mishandles-6th-arg.patch
net-inet_hashtablesh-needs-vmalloch.patch


Maintainers miss a lot of stuff :(



2007-11-13 04:45:17

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix the ustat() regression


On Nov 12, 2007, at 4:21 PM, Andrew Morton wrote:

> On Sat, 10 Nov 2007 17:02:19 -0600 (CST) Kumar Gala
> <[email protected]> wrote:
>
>> Since 2.6.18, the superblock sb->s_root has been a dummy dentry
>> with a
>> dummy inode. This breaks ustat(), which actually uses sb->s_root in a
>> vfstat() call.
>>
>> Fix this by making the s_root a dummy alias to the directory inode
>> that was
>> used when creating the superblock.
>
> I already have this in my
> things-which-need-to-go-into-2.6.24-via-maintainers queue.

Ok, didn't seen one of your normal emails about it going into -mm.
Anyways, want to make sure this finds its way in to 2.6.24

> That queue is huge:

when is one of your queue's not? :)

- k

> i-oat-add-support-for-version-2-of-ioatdma-device.patch
> reiserfs-dont-drop-pg_dirty-when-releasing-sub-page-sized-dirty-
> file.patch
> rtc-release-correct-region-in-error-path.patch
> rtc-fallback-to-requesting-only-the-ports-we-actually-use.patch
> i5000_edac-no-need-to-__stringify-kbuild_basename.patch
> serial-only-use-pnp-irq-if-its-valid.patch
> sunrpc-xprtrdma-transportc-fix-use-after-free.patch
> fix-mm-utilckrealloc.patch
> fuse_file_alloc-fix-null-dereferences.patch
> tle62x0-driver-stops-ignoring-read-errors.patch
> rd-fix-data-corruption-on-memory-pressure.patch
> cris-gpio-undo-locks-before-returning.patch
> mips-undo-locking-on-error-path-returns.patch
> mips-undo-locking-on-error-path-returns-checkpatch-fixes.patch
> nfs-fix-the-ustat-regression.patch
> proc-simplify-and-correct-proc_flush_task.patch
> fix-param_sysfs_builtin-name-length-check.patch
> rtc-convert-mutex-to-bitfield.patch
> mark-sys_open-sys_read-exports-unused.patch
> sysctl-fix-token-ring-procname.patch
> gbefb-fix-section-mismatch-warnings.patch
> vmstat-fix-section-mismatch-warning.patch
> pidns-place-under-config_experimental.patch
> pidns-place-under-config_experimental-checkpatch-fixes.patch
> __do_irq-does-not-check-irq_disabled-when-irq_per_cpu-is-set.patch
> hibernate-fix-lockdep-report-2.patch
> smbfs-fix-debug-builds.patch
> fix-64kb-blocksize-in-ext3-directories.patch
> fix-64kb-blocksize-in-ext3-directories-checkpatch-fixes.patch
> uml-fix-spurious-irq-testing.patch
> uml-remove-last-include-of-libc-asm-pageh.patch
> uml-fix-build-for-config_tcp.patch
> uml-fix-build-for-config_printk.patch
> markers-fix-warnings.patch
> swap-delay-accounting-include-lock_page-delays.patch
> file-capabilities-allow-sigcont-within-session-v2.patch
> file-capabilities-allow-sigcont-within-session-v2-checkpatch-
> fixes.patch
> file-capabilities-allow-sigcont-within-session-v2-file-capabilities-
> remove-the-non-matching-uid-special-case-for-kill.patch
> feature-removal-schedule-remove-sa_-flags-entry.patch
> kernel-taskstatsc-fix-bogus-nlmsg_free.patch
> x86-show-cpuinfo-only-for-online-cpus.patch
> make-proc-acpi-ac_adapter-dependent-on-acpi_procfs.patch
> acpi-ac-update-ac-state-on-resume.patch
> keyspan-init-termios-properly.patch
> x86-disable-preemption-in-delay_tsc.patch
> tty-fix-network-driver-interactions-with-tcget-set-checkpatch-
> fixes.patch
> tty-fix-tty-network-driver-interactions-with-tcget-tcset-calls-x86-
> fix.patch
> x86_64-fix-cpu-hotplug-regression.patch
> oprofile-fix-oops-on-x86-32-bit.patch
> x86-32-bit-ptrace-emulation-mishandles-6th-arg.patch
> net-inet_hashtablesh-needs-vmalloch.patch
>
>
> Maintainers miss a lot of stuff :(
>
>