Linux NFS development
 help / color / mirror / Atom feed
From: Olaf Kirch <okir@suse.de>
To: Neil Brown <neilb@cse.unsw.edu.au>
Cc: nfs@lists.sourceforge.net
Subject: Re: nfs3 dentry encoding
Date: Thu, 27 May 2004 14:15:45 +0200	[thread overview]
Message-ID: <20040527121545.GL12225@suse.de> (raw)
In-Reply-To: <16565.52349.592581.997725@cse.unsw.edu.au>

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

On Thu, May 27, 2004 at 09:09:49PM +1000, Neil Brown wrote:
> Hmmm, just as well someone is watching over me!  Thanks.

I'm currently looking into a failure with a Solaris 9 running connectathon
against a 2.6 Linux server. It fails in a test case doing a readdirplus
of a large directory, because it scribbles the offset value into
random memory locations (usually at page offset 0)

See the attached patch, which I also posted in a follow-up to
your message to to linux-kernel.

The first hunk is rather cosmetic; rq_res is cleared by a memset
so there's not need to zero the offset pointer(s) here. In fact
we don't initialize them in readdirplus anyway.

Olaf
-- 
Olaf Kirch     |  The Hardware Gods hate me.
okir@suse.de   |
---------------+ 

[-- Attachment #2: nfsd-encode-dirent3 --]
[-- Type: text/plain, Size: 2757 bytes --]

Index: linux-2.6.5/fs/nfsd/nfs3proc.c
===================================================================
--- linux-2.6.5.orig/fs/nfsd/nfs3proc.c	2004-05-27 12:22:43.000000000 +0200
+++ linux-2.6.5/fs/nfsd/nfs3proc.c	2004-05-27 12:34:40.000000000 +0200
@@ -437,7 +437,6 @@
 	resp->buflen = count;
 	resp->common.err = nfs_ok;
 	resp->buffer = argp->buffer;
-	resp->offset = NULL;
 	resp->rqstp = rqstp;
 	nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, 
 					&resp->common, nfs3svc_encode_entry);
Index: linux-2.6.5/fs/nfsd/nfs3xdr.c
===================================================================
--- linux-2.6.5.orig/fs/nfsd/nfs3xdr.c	2004-05-27 12:22:43.000000000 +0200
+++ linux-2.6.5/fs/nfsd/nfs3xdr.c	2004-05-27 12:39:09.000000000 +0200
@@ -887,8 +887,18 @@
 	int		elen;		/* estimated entry length in words */
 	int		num_entry_words = 0;	/* actual number of words */
 
-	if (cd->offset)
-		xdr_encode_hyper(cd->offset, (u64) offset);
+	if (cd->offset) {
+		u64 offset64 = offset;
+
+		if (unlikely(cd->offset1)) {
+			/* we ended up with offset on a page boundary */
+			*cd->offset = htonl(offset64 >> 32);
+			*cd->offset1 = htonl(offset64 & 0xffffffff);
+			cd->offset1 = NULL;
+		} else {
+			xdr_encode_hyper(cd->offset, (u64) offset);
+		}
+	}
 
 	/*
 	dprintk("encode_entry(%.*s @%ld%s)\n",
@@ -969,17 +979,32 @@
 			/* update offset */
 			cd->offset = cd->buffer + (cd->offset - tmp);
 		} else {
+			unsigned int offset_r = (cd->offset - tmp) << 2;
+
+			/* update pointer to offset location.
+			 * This is a 64bit quantity, so we need to
+			 * deal with 3 cases:
+			 *  -	entirely in first page
+			 *  -	entirely in second page
+			 *  -	4 bytes in each page
+			 */
+			if (offset_r + 8 <= len1) {
+				cd->offset = p + (cd->offset - tmp);
+			} else if (offset_r >= len1) {
+				cd->offset -= len1 >> 2;
+			} else {
+				/* sitting on the fence */
+				BUG_ON(offset_r != len1 - 4);
+				cd->offset = p + (cd->offset - tmp);
+				cd->offset1 = tmp;
+			}
+
 			len2 = (num_entry_words << 2) - len1;
 
 			/* move from temp page to current and next pages */
 			memmove(p, tmp, len1);
 			memmove(tmp, (caddr_t)tmp+len1, len2);
 
-			/* update offset */
-			if (((cd->offset - tmp) << 2) <= len1)
-				cd->offset = p + (cd->offset - tmp);
-			else
-				cd->offset -= len1 >> 2;
 			p = tmp + (len2 >> 2);
 		}
 	}
Index: linux-2.6.5/include/linux/nfsd/xdr3.h
===================================================================
--- linux-2.6.5.orig/include/linux/nfsd/xdr3.h	2004-05-27 12:22:43.000000000 +0200
+++ linux-2.6.5/include/linux/nfsd/xdr3.h	2004-05-27 12:28:20.000000000 +0200
@@ -183,6 +183,7 @@
 	u32 *			buffer;
 	int			buflen;
 	u32 *			offset;
+	u32 *			offset1;
 	struct svc_rqst *	rqstp;
 
 };

  reply	other threads:[~2004-05-27 12:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-05-18  1:31 [PATCH] kNFSd - 0 of 10 - Introduction NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 6 of 10 - Fix race conditions in idmapper NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 8 of 10 - Reduce timeout when waiting for idmapper userspace daemon NeilBrown
2004-05-27 10:20   ` nfs3 dentry encoding Olaf Kirch
2004-05-27 11:09     ` Neil Brown
2004-05-27 12:15       ` Olaf Kirch [this message]
2004-05-18  1:31 ` [PATCH] kNFSd - 4 of 10 - Change fh_compose to NOT consume a reference to the dentry NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 1 of 10 - Use correct _bh locking on sv_lock NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 9 of 10 - Remove check on number of threads waiting on user-space NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 3 of 10 - Allow larger writes to sunrpc/svc caches NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 10 of 10 - Add a warning when upcalls fail, NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 2 of 10 - Make sure CACHE_NEGATIVE is cleared when a cache entry is updates NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 5 of 10 - Protect reference to exp across calls to nfsd_cross_mnt NeilBrown
2004-05-18  1:31 ` [PATCH] kNFSd - 7 of 10 - Improve idmapper behaviour on failure NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040527121545.GL12225@suse.de \
    --to=okir@suse.de \
    --cc=neilb@cse.unsw.edu.au \
    --cc=nfs@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox