Readdirplus compatibility fix for IRIX server

Readdirplus compatibility fix for IRIX server

[Steve Dickson]

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

Hey Trond,

There is a compatibly issue with the linux-2.4.X-X-rdplus.dif
patch on your website and IRIX servers.

The scenario is:

mount irix:/server /mnt/irix
cd /mnt/irix;
mkdir dir; touch dir/file;
rm -rf dir # fails with an -525 error code (ETOOSMALL)

The problem happens when nfs_readdir() sends an "old"
cookie  (via uncached_readdir())

        res = readdir_search_pagecache(desc);
        if (res == -EBADCOOKIE) {
            /* This means either end of directory */
            if (desc->entry->cookie != desc->target) {
                /* Or that the server has 'lost' a cookie */
                res = uncached_readdir(desc, dirent, filldir);
                if (res >= 0)
                    continue;
            }

It seems IRIX servers fails READDIRPLUS calls with
NFSERR_TOOSMALL when the cookie does not exist which
is *not* the case with "normal" READDIR calls...
Cute uh...

Now one way to handle this is to simply ignore this
error case..
            if (res == -EIO)
                desc->error = 0;

since uncached_readdir() will always return -EIO when
the RPC call fails (which does work)....

But I got to wondering.... why is that last readdir RPC
even needed when the server *has* already set the EOF bit?

Since the server is telling client there is no more data,
is there really a need for that final readdir RPC? Now
if there is a cookie mismatch and the EOF bit is not set
then I (guess) can see the need for another RPC... But in the
"normal" case it appears that extra RPC is not needed....
(Note: This extra RPC happens with both READDIR and READDIRPLUSs
it does affect the stock kernels and it seems to happen with
non-linux servers, like netapp and IRIX)

The attached patch eliminates the extra RPC (when not
needed) which in turn takes care of the IRIX incompatibly
problem....

Comments?

SteveD.

[-- Attachment #2: linux-2.4.22-nfs-readdirplus-irix.patch --]
[-- Type: text/plain, Size: 557 bytes --]

--- linux-2.4.22/fs/nfs/dir.c.org	2003-12-09 14:21:59.000000000 -0500
+++ linux-2.4.22/fs/nfs/dir.c	2003-12-10 16:49:28.000000000 -0500
@@ -431,7 +431,8 @@ static int nfs_readdir(struct file *filp
 		res = readdir_search_pagecache(desc);
 		if (res == -EBADCOOKIE) {
 			/* This means either end of directory */
-			if (desc->entry->cookie != desc->target) {
+			if (desc->entry->eof == 0 &&
+				desc->entry->cookie != desc->target) {
 				/* Or that the server has 'lost' a cookie */
 				res = uncached_readdir(desc, dirent, filldir);
 				if (res >= 0)

Readdirplus compatibility fix for IRIX server

[Trond Myklebust]

>>>>> " " == Steve Dickson <SteveD@redhat.com> writes:

     > It seems IRIX servers fails READDIRPLUS calls with
     > NFSERR_TOOSMALL when the cookie does not exist which is *not*
     > the case with "normal" READDIR calls...  Cute uh...

Uhhh... Quite...

Now it is possible that we ought to fall back to using READDIR when
the server returns an NFSERR_TOOSMALL error, but the above blatant
server bug sure as hell isn't the reason. Rather it is the fact that
READDIR requires a smaller buffer, and so may help us recover from
*real* NFSERR_TOOSMALL errors.

     > Since the server is telling client there is no more data, is
     > there really a need for that final readdir RPC? Now if there is
     > a cookie mismatch and the EOF bit is not set then I (guess) can
     > see the need for another RPC... But in the "normal" case it
     > appears that extra RPC is not needed....  (Note: This extra RPC
     > happens with both READDIR and READDIRPLUSs it does affect the
     > stock kernels and it seems to happen with non-linux servers,
     > like netapp and IRIX)

     > The attached patch eliminates the extra RPC (when not needed)
     > which in turn takes care of the IRIX incompatibly problem....

     > Comments?

You are misinterpreting. entry->eof will *always* be set in this case,
because readdir_search_pagecache() will have had to iterate through
the entire page cache in order to determine that it could not find the
cookie.

The point is that when you do 'rm -rf', you are deleting files as you
iterate through the directory, and so the file entries are
disappearing from the page cache too. We use the readdir entry's
cookie as our file pointer, hence when that entry disappears from
underneath us, we lose our 'place marker' in the page cache.

At that point we have to ask the server for help: since the cookie can
be presented to the server in order to have it return the
corresponding next entry, we are thus able to recover.  Your patch is
merely short-circuiting that recovery mechanism.

Cheers,
  Trond


-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive?  Does it
help you create better code?  SHARE THE LOVE, and help us help
YOU!  Click Here: http://sourceforge.net/donate/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Readdirplus compatibility fix for IRIX server

[Greg Banks]

Trond Myklebust wrote:
> 
> >>>>> " " == Steve Dickson <SteveD@redhat.com> writes:
> 
>      > It seems IRIX servers fails READDIRPLUS calls with
>      > NFSERR_TOOSMALL when the cookie does not exist which is *not*
>      > the case with "normal" READDIR calls...  Cute uh...
> 
> Uhhh... Quite...
> 
> [...] the above blatant
> server bug [...]

I've raised an SGI bug report for this.  I expect it to be fixed in IRIX 6.5.24.

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.


-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive?  Does it
help you create better code?  SHARE THE LOVE, and help us help
YOU!  Click Here: http://sourceforge.net/donate/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: Readdirplus compatibility fix for IRIX server

[Steve Dickson]

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

Trond Myklebust wrote:

>Your patch is merely short-circuiting that recovery mechanism
>
Exactly.... I realized this as soon as I hit the Send button. :-(
Here is a patch that explicitly fixes the problem and only
effects READDIRPLUSs

SteveD


[-- Attachment #2: linux-2.4.22-nfs-readdirplus-irix.patch --]
[-- Type: text/plain, Size: 543 bytes --]

--- linux-2.4.22/fs/nfs/dir.c.org	2003-12-09 14:21:59.000000000 -0500
+++ linux-2.4.22/fs/nfs/dir.c	2003-12-11 05:50:26.000000000 -0500
@@ -437,6 +437,14 @@ static int nfs_readdir(struct file *filp
 				if (res >= 0)
 					continue;
 			}
+			/*
+			 * Some IRIX servers fail READDIRPLUS RPC calls
+			 * with NFSERR_TOOSMALL when a non-existent cookie 
+			 * is requested, which is a bug. So ignore that failure.
+			 */
+			if (desc->plus && desc->error == -ETOOSMALL)
+				desc->error = 0;
+
 			res = 0;
 			break;
 		} else if (res < 0)