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:

@@ -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.

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

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

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

-------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs