[PATCH]

[PATCH]

[Mark Hemment]

  For RPC over UDP, after receiving a packet kick another thread as soon
as possible.  This helps NFS performance.
  Patch is against 2.6.0-test3.

Mark


diff -urNp linux-2.6.0-test3/net/sunrpc/svcsock.c linux-2.6.0-test3-sunrpc/net/sunrpc/svcsock.c
--- linux-2.6.0-test3/net/sunrpc/svcsock.c	2003-08-09 05:32:33.000000000 +0100
+++ linux-2.6.0-test3-sunrpc/net/sunrpc/svcsock.c	2003-08-17 19:42:58.000000000 +0100
@@ -578,8 +578,14 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
 		/* possibly an icmp error */
 		dprintk("svc: recvfrom returned error %d\n", -err);
 	}
+	svsk->sk_sk->sk_stamp = skb->stamp;
 	set_bit(SK_DATA, &svsk->sk_flags); /* there may be more data... */

+	/*
+	 * Maybe more packets - kick another thread ASAP.
+	 */
+	svc_sock_received(svsk);
+
 	len  = skb->len - sizeof(struct udphdr);
 	rqstp->rq_arg.len = len;

@@ -590,8 +596,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
 	rqstp->rq_addr.sin_port = skb->h.uh->source;
 	rqstp->rq_addr.sin_addr.s_addr = skb->nh.iph->saddr;

-	svsk->sk_sk->sk_stamp = skb->stamp;
-
 	if (skb_is_nonlinear(skb)) {
 		/* we have to copy */
 		local_bh_disable();
@@ -599,7 +603,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
 			local_bh_enable();
 			/* checksum error */
 			skb_free_datagram(svsk->sk_sk, skb);
-			svc_sock_received(svsk);
 			return 0;
 		}
 		local_bh_enable();
@@ -611,7 +614,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
 		if (skb->ip_summed != CHECKSUM_UNNECESSARY) {
 			if ((unsigned short)csum_fold(skb_checksum(skb, 0, skb->len, skb->csum))) {
 				skb_free_datagram(svsk->sk_sk, skb);
-				svc_sock_received(svsk);
 				return 0;
 			}
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -631,9 +633,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
 	if (serv->sv_stats)
 		serv->sv_stats->netudpcnt++;

-	/* One down, maybe more to go... */
-	svc_sock_received(svsk);
-
 	return len;
 }




-------------------------------------------------------
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: [PATCH]

[Neil Brown]

On Monday August 18, markhe@veritas.com wrote:
>   For RPC over UDP, after receiving a packet kick another thread as soon
> as possible.  This helps NFS performance.
>   Patch is against 2.6.0-test3.
> 

Yep, I like this.  I'll send it on.

Have you measured any performance improvement, or it is just
theoretical?

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

[PATCH]:

[Steve Dickson]

[As requested, here is the debugging portion broken out of
the 6th patch in the "Dynamic Pseudo Root" patch series.]

Added dprintk to the top and bottom of both expkey_parse() and
svc_export_parse(). The top dprintks shows what rpc.mountd gave
to the routines to parse. These match up well with the current
debugging statements in the rpc.mount routines nfsd_export()
and nfsd_fh(). 

The bottom two dprintks show when either routine error out. This
was very useful in debugging why exports failed or hang.

I also added a number dprintks to the sunrpc caching code. Being
there was exactly one debugging statement through out the entire
chuck of code, it was literally impossible to tell whats going
on. This dprintks will show if and when upcalls are made and
(more importantly) when they are not.

All of these dprintks are not very busy code paths. They only 
pop when an exported is created or modified.

Signed-off-by: Steve Dickson <steved@redhat.com>

diff -up linux/fs/nfsd/export.c.org linux/fs/nfsd/export.c
--- linux/fs/nfsd/export.c.org	2008-10-24 13:00:36.000000000 -0400
+++ linux/fs/nfsd/export.c	2008-10-24 12:59:46.000000000 -0400
@@ -104,6 +104,7 @@ static int expkey_parse(struct cache_det
 	if (mesg[mlen-1] != '\n')
 		return -EINVAL;
 	mesg[mlen-1] = 0;
+	dprintk("expkey_parse: '%s'\n", mesg);
 
 	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	err = -ENOMEM;
@@ -182,6 +183,8 @@ static int expkey_parse(struct cache_det
 	if (dom)
 		auth_domain_put(dom);
 	kfree(buf);
+	if (err)
+		dprintk("expkey_parse: err %d\n", err);
 	return err;
 }
 
@@ -353,6 +356,8 @@ static void svc_export_request(struct ca
 		return;
 	}
 	qword_add(bpp, blen, pth);
+	dprintk("svc_export_request: pth %s\n", pth);
+
 	(*bpp)[-1] = '\n';
 }
 
@@ -520,6 +525,7 @@ static int svc_export_parse(struct cache
 	if (mesg[mlen-1] != '\n')
 		return -EINVAL;
 	mesg[mlen-1] = 0;
+	dprintk("svc_export_parse: '%s'\n", mesg);
 
 	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	err = -ENOMEM;
@@ -632,6 +638,8 @@ static int svc_export_parse(struct cache
 	if (dom)
 		auth_domain_put(dom);
 	kfree(buf);
+	if (err)
+		dprintk("svc_export_parse: err %d\n", err);
 	return err;
 }
 
diff -up linux/net/sunrpc/cache.c.org linux/net/sunrpc/cache.c
--- linux/net/sunrpc/cache.c.org	2008-10-24 13:00:46.000000000 -0400
+++ linux/net/sunrpc/cache.c	2008-10-14 07:54:01.000000000 -0400
@@ -215,11 +215,13 @@ int cache_check(struct cache_detail *det
 		if (rv == -EAGAIN)
 			rv = -ENOENT;
 	} else if (rv == -EAGAIN || age > refresh_age/2) {
-		dprintk("RPC:       Want update, refage=%ld, age=%ld\n",
-				refresh_age, age);
+		dprintk("check_check: upcall: h 0x%p pending %d rv %d\n", 
+			h, test_bit(CACHE_PENDING, &h->flags), rv);
 		if (!test_and_set_bit(CACHE_PENDING, &h->flags)) {
 			switch (cache_make_upcall(detail, h)) {
 			case -EINVAL:
+				dprintk("check_check: upcall(-EINVAL): h 0x%p rv %d\n",
+					h, rv);
 				clear_bit(CACHE_PENDING, &h->flags);
 				if (rv == -EAGAIN) {
 					set_bit(CACHE_NEGATIVE, &h->flags);
@@ -231,6 +233,8 @@ int cache_check(struct cache_detail *det
 
 			case -EAGAIN:
 				clear_bit(CACHE_PENDING, &h->flags);
+				dprintk("check_check: upcall(-EAGAIN): h 0x%p flags 0x%lx\n",
+					h, h->flags);
 				cache_revisit_request(h);
 				break;
 			}
@@ -560,13 +564,16 @@ static int cache_defer_req(struct cache_
 		/* too much in the cache, randomly drop this one,
 		 * or continue and drop the oldest below
 		 */
-		if (net_random()&1)
+		if (net_random()&1) {
+			dprintk("cache_defer_req:  0x%p: dropping request\n", item);
 			return -ETIMEDOUT;
+		}
 	}
 	dreq = req->defer(req);
-	if (dreq == NULL)
+	if (dreq == NULL) {
+		dprintk("cache_defer_req: 0x%p: request timedout\n", item);
 		return -ETIMEDOUT;
-
+	}
 	dreq->item = item;
 
 	spin_lock(&cache_defer_lock);
@@ -596,6 +603,7 @@ static int cache_defer_req(struct cache_
 		/* must have just been validated... */
 		cache_revisit_request(item);
 	}
+	dprintk("cache_defer_req:  0x%p: request deferred\n", item);
 	return 0;
 }

Re: [PATCH]:

[J. Bruce Fields]

On Fri, Oct 24, 2008 at 01:31:57PM -0400, Steve Dickson wrote:
> [As requested, here is the debugging portion broken out of
> the 6th patch in the "Dynamic Pseudo Root" patch series.]
> 
> Added dprintk to the top and bottom of both expkey_parse() and
> svc_export_parse(). The top dprintks shows what rpc.mountd gave
> to the routines to parse. These match up well with the current
> debugging statements in the rpc.mount routines nfsd_export()
> and nfsd_fh(). 
>
> The bottom two dprintks show when either routine error out. This
> was very useful in debugging why exports failed or hang.


Did you try experiment with strace very much before trying this?
Something like

	strace -e read,write -s 1000 -p `pidof rpc.gssd`

will show the contents of the upcalls and downcalls and any returned
error, so I'm not convinced that dprintk's of the upcall/downcall data
are necessary.

> diff -up linux/net/sunrpc/cache.c.org linux/net/sunrpc/cache.c
> --- linux/net/sunrpc/cache.c.org	2008-10-24 13:00:46.000000000 -0400
> +++ linux/net/sunrpc/cache.c	2008-10-14 07:54:01.000000000 -0400
> @@ -215,11 +215,13 @@ int cache_check(struct cache_detail *det
>  		if (rv == -EAGAIN)
>  			rv = -ENOENT;
>  	} else if (rv == -EAGAIN || age > refresh_age/2) {
> -		dprintk("RPC:       Want update, refage=%ld, age=%ld\n",
> -				refresh_age, age);
> +		dprintk("check_check: upcall: h 0x%p pending %d rv %d\n", 
> +			h, test_bit(CACHE_PENDING, &h->flags), rv);

THis is actually changing the information printed instead of adding
more.  Are you sure this is what you want?

>  		if (!test_and_set_bit(CACHE_PENDING, &h->flags)) {
>  			switch (cache_make_upcall(detail, h)) {
>  			case -EINVAL:
> +				dprintk("check_check: upcall(-EINVAL): h 0x%p rv %d\n",
> +					h, rv);

>From a quick check of cache_make_upcall, -EINVAL is returned either when
cache_request is NULL (looks like that would be a bug?), or in the case
where nobody's listening on the given upcall channel, in which case
there's already a printk().

--b.

>  				clear_bit(CACHE_PENDING, &h->flags);
>  				if (rv == -EAGAIN) {
>  					set_bit(CACHE_NEGATIVE, &h->flags);
> @@ -231,6 +233,8 @@ int cache_check(struct cache_detail *det
>  
>  			case -EAGAIN:
>  				clear_bit(CACHE_PENDING, &h->flags);
> +				dprintk("check_check: upcall(-EAGAIN): h 0x%p flags 0x%lx\n",
> +					h, h->flags);
>  				cache_revisit_request(h);
>  				break;
>  			}
> @@ -560,13 +564,16 @@ static int cache_defer_req(struct cache_
>  		/* too much in the cache, randomly drop this one,
>  		 * or continue and drop the oldest below
>  		 */
> -		if (net_random()&1)
> +		if (net_random()&1) {
> +			dprintk("cache_defer_req:  0x%p: dropping request\n", item);
>  			return -ETIMEDOUT;
> +		}
>  	}
>  	dreq = req->defer(req);
> -	if (dreq == NULL)
> +	if (dreq == NULL) {
> +		dprintk("cache_defer_req: 0x%p: request timedout\n", item);
>  		return -ETIMEDOUT;
> -
> +	}
>  	dreq->item = item;
>  
>  	spin_lock(&cache_defer_lock);
> @@ -596,6 +603,7 @@ static int cache_defer_req(struct cache_
>  		/* must have just been validated... */
>  		cache_revisit_request(item);
>  	}
> +	dprintk("cache_defer_req:  0x%p: request deferred\n", item);
>  	return 0;
>  }
>  
> _______________________________________________
> NFSv4 mailing list
> NFSv4@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4

Re: [PATCH]:

[Steve Dickson]

J. Bruce Fields wrote:
> On Fri, Oct 24, 2008 at 01:31:57PM -0400, Steve Dickson wrote:
>> [As requested, here is the debugging portion broken out of
>> the 6th patch in the "Dynamic Pseudo Root" patch series.]
>>
>> Added dprintk to the top and bottom of both expkey_parse() and
>> svc_export_parse(). The top dprintks shows what rpc.mountd gave
>> to the routines to parse. These match up well with the current
>> debugging statements in the rpc.mount routines nfsd_export()
>> and nfsd_fh(). 
>>
>> The bottom two dprintks show when either routine error out. This
>> was very useful in debugging why exports failed or hang.
> 
> 
> Did you try experiment with strace very much before trying this?
> Something like
> 
> 	strace -e read,write -s 1000 -p `pidof rpc.gssd`
> 
> will show the contents of the upcalls and downcalls and any returned
> error, so I'm not convinced that dprintk's of the upcall/downcall data
> are necessary.
> 
No, I have not, but it does look interesting and I'll give it try. 

But I also think its important to have one united debugging interface
that gives meaningful information when its turned on. 

For example, today you turn on export debugging with
    rpcdebug -m nfsd -s export 

you get a couple of "found this", "path is that" message that are 
basically meaningless and you have no idea where they are coming from.

When you turn on the cache debugging via:
    rpcdebug -m nfsd -s cache

you get absolutely nothing, since there is exactly one dprintk() in all
that code that I could never get to pop... 

So with my proposed dprinks it makes those commands much more meaningful and
useful by adding straightforward debugging strings that identify where they
are in the kernel. They also tie in nicely with the syslog entries that currently 
come out of the mountd debugging. Also they are not in a high traffic area. Meaning
they only pop when during exports and mounts, unlike an I/O path...

Maybe I'm don't understand the current debugging philosophy, but if a couple
non-intrusive dprintk make the debugging commands a bit more useful, why
isn't that a good thing?

steved.

[PATCH]

[Lu, Xinyu]

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

 nfs4.0 server st_write: fix the amount of data sent in the testLargeData
    
    The maximum amount of data could be writen is NFSSVC_MAXBLKSIZE. The value of NFSSVC_MAXB
LKSIZE defined in the kernel is RPCSVC_MAXPLAYLOAD. If the value written exceeds NFSSVC_MAXBL
KSIZE, the value is fixed as the value of NFSSVC_MAXBLKSIZE. The value of RPCSVC_MAXPAYLOAD i
s 1*1024*1024u and "abcdefghijklmnopq"*0x10000 exceeds it. So the previous test is bound to f
ail and is meaningless.

    Signed-off-by: Lu Xinyu <luxy.fnst@cn.fujitsu.com>

diff --git a/nfs4.0/servertests/st_write.py b/nfs4.0/servertests/st_write.py
index 710452e..a7dae03 100644
--- a/nfs4.0/servertests/st_write.py
+++ b/nfs4.0/servertests/st_write.py
@@ -130,7 +130,7 @@ def testLargeData(t, env):
     c = env.c1
     c.init_connection()
     fh, stateid = c.create_confirm(t.code)
-    data = "abcdefghijklmnopq" * 0x10000
+    data = "a" * 1024 * 1024
     # Write the data
     pos = 0
     while pos < len(data):





[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-nfs4.0-server-st_write-fix-the-amount-of-data-sent-i.patch --]
[-- Type: text/x-patch; name="0001-nfs4.0-server-st_write-fix-the-amount-of-data-sent-i.patch", Size: 1270 bytes --]

>From 6adc3da0ab17eb7e52b47805e6999d65b043fa7f Mon Sep 17 00:00:00 2001
From: Lu Xinyu <luxy.fnst@cn.fujitsu.com>
Date: Thu, 30 Nov 2017 13:24:15 +0800
Subject: [PATCH] nfs4.0 server st_write: fix the amount of data sent in the
 testLargeData

The maximum amount of data could be writen is NFSSVC_MAXBLKSIZE. The value of NFSSVC_MAXBLKSIZE defined in the kernel is RPCSVC_MAXPLAYLOAD. If the value written exceeds NFSSVC_MAXBLKSIZE, the value is fixed as the value of NFSSVC_MAXBLKSIZE. The value of RPCSVC_MAXPAYLOAD is 1*1024*1024u and "abcdefghijklmnopq"*0x10000 exceeds it. So the previous test is bound to fail and is meaningless.

Signed-off-by: Lu Xinyu <luxy.fnst@cn.fujitsu.com>
---
 nfs4.0/servertests/st_write.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nfs4.0/servertests/st_write.py b/nfs4.0/servertests/st_write.py
index 710452e..a7dae03 100644
--- a/nfs4.0/servertests/st_write.py
+++ b/nfs4.0/servertests/st_write.py
@@ -130,7 +130,7 @@ def testLargeData(t, env):
     c = env.c1
     c.init_connection()
     fh, stateid = c.create_confirm(t.code)
-    data = "abcdefghijklmnopq" * 0x10000
+    data = "a" * 1024 * 1024
     # Write the data
     pos = 0
     while pos < len(data):
-- 
2.13.3