Readdirplus compatibility fix for IRIX server

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.

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

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

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

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

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