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

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

[Steve Dickson]

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.

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

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 127 bytes --]

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

[Neil Brown]

On Wednesday August 6, arjanv@redhat.com 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


-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Neil Brown]

On Wednesday August 6, arjanv@redhat.com 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

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

[Rik van Riel]

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

Re: [PATCH] kNFSd: Fixes a problem with inode clean up for vxfs

[Neil Brown]

On Wednesday August 6, SteveD@redhat.com 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.
> 


-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

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

[Neil Brown]

On Wednesday August 6, SteveD@redhat.com 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.
>