Re: [PATCH v2 02/12] nfsd41: sunrpc: Added rpc server-side backchannel handling

[Benny Halevy <bhalevy@panasas.com>]

On Sep. 10, 2009, 14:49 +0300, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > > index f412a85..f577e5a 100644
> > > --- a/net/sunrpc/xprt.c
> > > +++ b/net/sunrpc/xprt.c
> > > @@ -832,6 +832,11 @@ static void xprt_timer(struct rpc_task *task)
> > >  	spin_unlock_bh(&xprt->transport_lock);
> > >  }
> > >  
> > > +static inline int xprt_has_timer(struct rpc_xprt *xprt)
> > > +{
> > > +	return xprt->idle_timeout != (~0);
> > > +}
> 
> Why did this change again?
> 
> It's a disconnect timer, and the idle_timeout sets the timeout period. A
> test for whether or not that period is 0 therefore makes sense (a zero
> timeout being a nonsense value for a timer).
> 
> Testing for arbitrary non-zero values is more dubious, and forces the
> backchannel to explicitly set a non-zero value. What value does that
> add?
> 

Good question. I agree with your direction.

Alexandros, why was this != 0 in PATCH 3/3 v2:
http://linux-nfs.org/pipermail/pnfs/2009-September/009057.html
but changed back to ~0 in PATCH 3/3 v2.1?
http://linux-nfs.org/pipermail/pnfs/2009-September/009059.html

With this in mind, xs_setup_bc_tcp can simply initialize idle_timeout
to zero, right?
 	xprt->bind_timeout = 0;
 	xprt->connect_timeout = 0;
 	xprt->reestablish_timeout = 0;
-	xprt->idle_timeout = (~0);
+	xprt->idle_timeout = 0;
 
 	/*
 	 * The backchannel uses the same socket connection as the

Benny

> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
>