2003-08-06 16:11:14

by Steve Dickson

[permalink] [raw]
Subject: [NFS] [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

This a patch I've received from Veritas. Supposedly they have
already submitted this but I can't seem to find it in any 2.4 trees..

Does anybody recognize this and are there any known issues with it?

The Problem: The nfsd_findparent creates a dentry using d_alloc_root.
The d_op
vector pointer in this dentry is not initialized. Hence filesystems that
supply
the vector have a problem. nfs exports of such filesystems do not work
correctly under memory pressure. vxfs, vfat, ntfs are amongst the
filesystems
affected by the bug. Need redhat to fix nfsd code in their kernels.
Ideally
a kernel needs to ask a filesystem to setup a d_op vector. An entry point
into a filesystem for doing this job doesn't exist. We can work around the
problem by copying d_op vector pointer from the child of the dentry, whose
d_op vector is correct.


The Patch:

--- ./fs/nfsd/nfsfh.c.diff Wed Jul 2 13:17:35 2003
+++ ./fs/nfsd/nfsfh.c Tue Jul 29 04:45:43 2003
@@ -303,6 +303,7 @@ struct dentry *nfsd_findparent(struct de
if (pdentry) {
igrab(tdentry->d_inode);
pdentry->d_flags |=
DCACHE_NFSD_DISCONNECTED;
+ pdentry->d_op = child->d_op;
}
}
if (pdentry == NULL)

SteveD.



2003-08-06 16:29:30

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [NFS] [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

On Wed, 2003-08-06 at 18:11, Steve Dickson wrote:
> , vfat, ntfs

you can't NFS export vfat..... for lots of other reasons


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-08-06 17:08:54

by Rik van Riel

[permalink] [raw]
Subject: Re: [NFS] [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

On Wed, 6 Aug 2003, Steve Dickson wrote:

> This a patch I've received from Veritas. Supposedly they have
> already submitted this but I can't seem to find it in any 2.4 trees..
>
> Does anybody recognize this and are there any known issues with it?

It makes me wonder what is so special about vxfs that they need
to modify GPL code in order for it to work ...

Not that I'm against this change in principle, but I'd just like
it to be useful for GPL software too, otherwise it'd just be a
hook for non-GPL software and a fine line to a GPL violation.

--
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan

2003-08-06 23:32:40

by NeilBrown

[permalink] [raw]
Subject: Re: [NFS] [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

On Wednesday August 6, [email protected] wrote:
> On Wed, 2003-08-06 at 18:11, Steve Dickson wrote:
> > , vfat, ntfs
>
> you can't NFS export vfat..... for lots of other reasons

Have you tried?

It is certainly not bullet-proof, but it works in simple cases (but
probably not in cases where the patch in question becomes an issue).

NeilBrown

2003-08-06 23:29:54

by NeilBrown

[permalink] [raw]
Subject: Re: [NFS] [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

On Wednesday August 6, [email protected] wrote:
> This a patch I've received from Veritas. Supposedly they have
> already submitted this but I can't seem to find it in any 2.4 trees..
>
> Does anybody recognize this and are there any known issues with it?

The patch is probably ok.
Both the current code and the new code are "wrong" as they assume
something about the setting of d_op, which only the filesystem could
know.
The current code assumes it will always be NULL.
The new code assumes it will be uniform within the filesystem.

Neither of these are certain to be true, but the later covers all
filesystems that the former covers and more, so it is safer.

It is not tecnically necessary as any filesystem in free to define
their fh_to_dentry operation to return a dentry that was not
DCACHE_NFSD_DISCONNECTED, and then this code would never be called.
The easiest way to write a fh_to_dentry that did this would be to copy
slabs of code out of nfsd/nfsfh.c, but there might be GPL issues if
Veritas did that.

All this is handled quite differently in 2.6 so it isn't an issue
there.

I would say "accept the patch". I might even submit it for 2.4.23...

NeilBrown



>
> The Problem: The nfsd_findparent creates a dentry using d_alloc_root.
> The d_op
> vector pointer in this dentry is not initialized. Hence filesystems that
> supply
> the vector have a problem. nfs exports of such filesystems do not work
> correctly under memory pressure. vxfs, vfat, ntfs are amongst the
> filesystems
> affected by the bug. Need redhat to fix nfsd code in their kernels.
> Ideally
> a kernel needs to ask a filesystem to setup a d_op vector. An entry point
> into a filesystem for doing this job doesn't exist. We can work around the
> problem by copying d_op vector pointer from the child of the dentry, whose
> d_op vector is correct.
>
>
> The Patch:
>
> --- ./fs/nfsd/nfsfh.c.diff Wed Jul 2 13:17:35 2003
> +++ ./fs/nfsd/nfsfh.c Tue Jul 29 04:45:43 2003
> @@ -303,6 +303,7 @@ struct dentry *nfsd_findparent(struct de
> if (pdentry) {
> igrab(tdentry->d_inode);
> pdentry->d_flags |=
> DCACHE_NFSD_DISCONNECTED;
> + pdentry->d_op = child->d_op;
> }
> }
> if (pdentry == NULL)
>
> SteveD.
>