diff mbox

[2/5] SUNRPC: allow disabling idle timeout

Message ID 20130424150037.GA20275@fieldses.org (mailing list archive)
State New, archived
Headers show

Commit Message

J. Bruce Fields April 24, 2013, 3 p.m. UTC
On Thu, Apr 18, 2013 at 10:25:49AM -0700, Chuck Lever wrote:
> 
> On Apr 18, 2013, at 10:14 AM, "J. Bruce Fields" <bfields@redhat.com> wrote:
> 
> > On Thu, Apr 18, 2013 at 05:07:03PM +0000, Myklebust, Trond wrote:
> >> On Thu, 2013-04-18 at 13:00 -0400, J. Bruce Fields wrote:
> >>> On Mon, Apr 15, 2013 at 03:35:04PM -0400, J. Bruce Fields wrote:
> >>>> From: "J. Bruce Fields" <bfields@redhat.com>
> >>>> 
> >>>> In the gss-proxy case we don't want to have to reconnect at random--we
> >>>> want to connect only on gss-proxy startup when we can steal gss-proxy's
> >>>> context to do the connect in the right namespace.
> >>>> 
> >>>> So, provide a flag that allows the rpc_create caller to turn off the
> >>>> idle timeout.
> >>> 
> >>> Chuck, the basic ideas was your suggestion, does the executation look OK
> >>> here?  I had to copy the rpc_create flags down to the xprt_create, I
> >>> don't know if that's reasonable.
> >> 
> >> This patch will conflict with commit
> >> b7993cebb841b0da7a33e9d5ce301a9fd3209165 (SUNRPC: Allow rpc_create() to
> >> request that TCP slots be unlimited) that was posted on this list
> >> earlier this week.
> > 
> > Oh, sorry, I missed that.
> > 
> > Presumably then I should just work on top of that and do the same
> > thing--define a pair of flags
> > {RP_CLNT_CREATE|XPRT_CREATE}_NO_IDLE_TIMEOUT and translate between the
> > two in rpc_create.
> 
> Agree.

The result (untested) looks like this.

If this is OK--Trond, do you mind if I merge this commit (or
nfs-for-next) into my tree, and then the rest of the gss-proxy patches
on top?

Or is the nfs-for-next branch still potentially subject to rewriting?

--b.

commit 17728da224288679811a9b305ae1211c3c7a6e7e
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Apr 11 15:06:36 2013 -0400

    SUNRPC: allow disabling idle timeout
    
    In the gss-proxy case we don't want to have to reconnect at random--we
    want to connect only on gss-proxy startup when we can steal gss-proxy's
    context to do the connect in the right namespace.
    
    So, provide a flag that allows the rpc_create caller to turn off the
    idle timeout.
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Trond Myklebust April 24, 2013, 3:03 p.m. UTC | #1
On Wed, 2013-04-24 at 11:00 -0400, J. Bruce Fields wrote:
> On Thu, Apr 18, 2013 at 10:25:49AM -0700, Chuck Lever wrote:
> > 
> > On Apr 18, 2013, at 10:14 AM, "J. Bruce Fields" <bfields@redhat.com> wrote:
> > 
> > > On Thu, Apr 18, 2013 at 05:07:03PM +0000, Myklebust, Trond wrote:
> > >> On Thu, 2013-04-18 at 13:00 -0400, J. Bruce Fields wrote:
> > >>> On Mon, Apr 15, 2013 at 03:35:04PM -0400, J. Bruce Fields wrote:
> > >>>> From: "J. Bruce Fields" <bfields@redhat.com>
> > >>>> 
> > >>>> In the gss-proxy case we don't want to have to reconnect at random--we
> > >>>> want to connect only on gss-proxy startup when we can steal gss-proxy's
> > >>>> context to do the connect in the right namespace.
> > >>>> 
> > >>>> So, provide a flag that allows the rpc_create caller to turn off the
> > >>>> idle timeout.
> > >>> 
> > >>> Chuck, the basic ideas was your suggestion, does the executation look OK
> > >>> here?  I had to copy the rpc_create flags down to the xprt_create, I
> > >>> don't know if that's reasonable.
> > >> 
> > >> This patch will conflict with commit
> > >> b7993cebb841b0da7a33e9d5ce301a9fd3209165 (SUNRPC: Allow rpc_create() to
> > >> request that TCP slots be unlimited) that was posted on this list
> > >> earlier this week.
> > > 
> > > Oh, sorry, I missed that.
> > > 
> > > Presumably then I should just work on top of that and do the same
> > > thing--define a pair of flags
> > > {RP_CLNT_CREATE|XPRT_CREATE}_NO_IDLE_TIMEOUT and translate between the
> > > two in rpc_create.
> > 
> > Agree.
> 
> The result (untested) looks like this.
> 
> If this is OK--Trond, do you mind if I merge this commit (or
> nfs-for-next) into my tree, and then the rest of the gss-proxy patches
> on top?
> 
> Or is the nfs-for-next branch still potentially subject to rewriting?

nfs-for-next is stable, so it should be safe to pull into your nfsd
tree.
J. Bruce Fields April 26, 2013, 3:43 p.m. UTC | #2
On Wed, Apr 24, 2013 at 03:03:23PM +0000, Myklebust, Trond wrote:
> On Wed, 2013-04-24 at 11:00 -0400, J. Bruce Fields wrote:
> > On Thu, Apr 18, 2013 at 10:25:49AM -0700, Chuck Lever wrote:
> > > 
> > > On Apr 18, 2013, at 10:14 AM, "J. Bruce Fields" <bfields@redhat.com> wrote:
> > > 
> > > > On Thu, Apr 18, 2013 at 05:07:03PM +0000, Myklebust, Trond wrote:
> > > >> On Thu, 2013-04-18 at 13:00 -0400, J. Bruce Fields wrote:
> > > >>> On Mon, Apr 15, 2013 at 03:35:04PM -0400, J. Bruce Fields wrote:
> > > >>>> From: "J. Bruce Fields" <bfields@redhat.com>
> > > >>>> 
> > > >>>> In the gss-proxy case we don't want to have to reconnect at random--we
> > > >>>> want to connect only on gss-proxy startup when we can steal gss-proxy's
> > > >>>> context to do the connect in the right namespace.
> > > >>>> 
> > > >>>> So, provide a flag that allows the rpc_create caller to turn off the
> > > >>>> idle timeout.
> > > >>> 
> > > >>> Chuck, the basic ideas was your suggestion, does the executation look OK
> > > >>> here?  I had to copy the rpc_create flags down to the xprt_create, I
> > > >>> don't know if that's reasonable.
> > > >> 
> > > >> This patch will conflict with commit
> > > >> b7993cebb841b0da7a33e9d5ce301a9fd3209165 (SUNRPC: Allow rpc_create() to
> > > >> request that TCP slots be unlimited) that was posted on this list
> > > >> earlier this week.
> > > > 
> > > > Oh, sorry, I missed that.
> > > > 
> > > > Presumably then I should just work on top of that and do the same
> > > > thing--define a pair of flags
> > > > {RP_CLNT_CREATE|XPRT_CREATE}_NO_IDLE_TIMEOUT and translate between the
> > > > two in rpc_create.
> > > 
> > > Agree.
> > 
> > The result (untested) looks like this.
> > 
> > If this is OK--Trond, do you mind if I merge this commit (or
> > nfs-for-next) into my tree, and then the rest of the gss-proxy patches
> > on top?
> > 
> > Or is the nfs-for-next branch still potentially subject to rewriting?
> 
> nfs-for-next is stable, so it should be safe to pull into your nfsd
> tree.

OK, done locally, pushing out that and the gss-proxy work soon pending
some testing.

This just means I'll want to wait till after your branch is pulled for
the next merge window before sending my pull-request, to avoid creating
confusion about who did what.

Therefore, please delay your request as long as you like, as that will
give me an excuse for my own procrastination.  Thanks!

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index e7d492c..bfe11be 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -125,6 +125,7 @@  struct rpc_create_args {
 #define RPC_CLNT_CREATE_DISCRTRY	(1UL << 5)
 #define RPC_CLNT_CREATE_QUIET		(1UL << 6)
 #define RPC_CLNT_CREATE_INFINITE_SLOTS	(1UL << 7)
+#define RPC_CLNT_CREATE_NO_IDLE_TIMEOUT	(1UL << 8)
 
 struct rpc_clnt *rpc_create(struct rpc_create_args *args);
 struct rpc_clnt	*rpc_bind_new_program(struct rpc_clnt *,
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index ff53924..cec7b9b 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -256,6 +256,7 @@  static inline int bc_prealloc(struct rpc_rqst *req)
 #endif /* CONFIG_SUNRPC_BACKCHANNEL */
 
 #define XPRT_CREATE_INFINITE_SLOTS	(1U)
+#define XPRT_CREATE_NO_IDLE_TIMEOUT	(1U << 1)
 
 struct xprt_create {
 	int			ident;		/* XPRT_TRANSPORT identifier */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 651245a..80cf232 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -416,6 +416,8 @@  struct rpc_clnt *rpc_create(struct rpc_create_args *args)
 
 	if (args->flags & RPC_CLNT_CREATE_INFINITE_SLOTS)
 		xprtargs.flags |= XPRT_CREATE_INFINITE_SLOTS;
+	if (args->flags & RPC_CLNT_CREATE_NO_IDLE_TIMEOUT)
+		xprtargs.flags |= XPRT_CREATE_NO_IDLE_TIMEOUT;
 	/*
 	 * If the caller chooses not to specify a hostname, whip
 	 * up a string representation of the passed-in address.
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 745fca3..095363e 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1300,6 +1300,8 @@  found:
 				-PTR_ERR(xprt));
 		goto out;
 	}
+	if (args->flags & XPRT_CREATE_NO_IDLE_TIMEOUT)
+		xprt->idle_timeout = 0;
 	INIT_WORK(&xprt->task_cleanup, xprt_autoclose);
 	if (xprt_has_timer(xprt))
 		setup_timer(&xprt->timer, xprt_init_autodisconnect,