[pnfs] [PATCH 12/17] Reroute a callback to be processed by the callback service.

[Iyer, Rahul]

> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy at panasas.com] 
> Sent: Monday, May 21, 2007 6:27 AM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [PATCH 12/17] Reroute a callback to be processed 
> by the callback service.
> 
> Iyer, Rahul wrote:
> 
> > From 205cbe5b3b6371126973ba1a682b0b922c1d9195 Mon Sep 17 
> 00:00:00 2001
> > Message-Id: 
> <205cbe5b3b6371126973ba1a682b0b922c1d9195.1179276403.git.iyer@
> netapp.com>
> > In-Reply-To: 
> <19b0fc3bc27cc1b1527dd685f71b79cd249a3779.1179276402.git.iyer@
> netapp.com>
> > References: 
> <19b0fc3bc27cc1b1527dd685f71b79cd249a3779.1179276402.git.iyer@
> netapp.com>
> > From: Rahul Iyer <iyer at netapp.com>
> > Date: Tue, 15 May 2007 16:57:28 -0700
> > Subject: [PATCH 12/17] Reroute a callback to be processed 
> by the callback service.
> > 
> > This routine is the heart of the mux/demux of the 
> callbacks. When a RPC request
> > arrives on the channel and is determined to be a callback, 
> the following thinfs
> > happen:
> > 
> > 1. A request (rpc_rqst struct) is allocated for the rpc 
> request and the request
> > is read in.
> > 
> > 2. The request is queued onto the callback request list of 
> the corresponding
> > svc_serv for processing by the callback service
> > 
> > Signed-off-by: Rahul Iyer <iyer at netapp.com>
> > ---
> >  net/sunrpc/xprtsock.c |   30 ++++++++++++++++++++++++------
> >  1 files changed, 24 insertions(+), 6 deletions(-)
> > 
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 50e000e..0c0d7e0 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/tcp.h>
> >  #include <linux/sunrpc/clnt.h>
> >  #include <linux/sunrpc/sched.h>
> > +#include <linux/sunrpc/bc_xprt.h>
> >  #include <linux/file.h>
> >  
> >  #include <net/sock.h>
> > @@ -689,11 +690,20 @@ static inline void 
> xs_tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc
> >  	spin_lock(&xprt->transport_lock);
> >  	calldir = ntohl(xprt->tcp_calldir);
> >  	if (calldir == RPC_CALL) {
> > -		dprintk("Callback received on backchannel!\n");
> > -		spin_unlock(&xprt->transport_lock);
> >  
> > -		/* Ignore callbacks for now */
> > -		return;
> > +		req = xprt_alloc_bc_request(xprt);
> > +
> > +		if (!req) {
> > +			printk(KERN_NOTICE "Couldn't get 
> rpc_rqst for the allback! Dropping callback...\n");
> > +			xprt->tcp_flags &= ~XPRT_COPY_DATA;
> > +			spin_unlock(&xprt->transport_lock);
> > +
> > +			/* 
> > +			 * Ignore the callback because we 
> couldn't allocate a
> > +			 * request
> > +			 */
> > +			return;
> > +		}
> 
> The spec says you should close the connection in this case
> assuming returning an appropriate error here is too 
> complicated for now.
> In any case the client MUST NOT silently drop a request
> (and a printk counts as silence from the server POV ;-)
> 
> Benny
> 
Ok. A possible solution is to set the size of the mempool to
ca_maxrequests. Any allocation that fails means that the ca_maxrequests
has been exceeded (though exceeding the limit doesn't mean allocation
failure, because it is a mempool). In this scenario, we can alert the
xprt that this happened and return an error. I can look at this.
Regards
Rahul