On Thu, Mar 18, 2010 at 11:08:18AM -0400, J. Bruce Fields wrote:
> On Wed, Mar 10, 2010 at 02:05:45PM -0500, J. Bruce Fields wrote:
> > On Tue, Mar 09, 2010 at 08:03:00PM -0500, Trond Myklebust wrote:
> > > Why should the back channel socket hold a reference to the svc_sock? The
> > > process that owns both of these things is an RPC server process. It
> > > already holds a reference to the svc_sock and presumably also to the
> > > back channel socket.
> >
> > Only while it's actually processing an rpc, I assume.
> >
> > I'll take a harder look at what should be the lifetime of the server
> > socket that the backchannel requests use.
> >
> > By the way, we also need to make sure the nfs server code gets some kind
> > of call when the client closes the connection, so we can set
> > callback-down session flags appropriately.
>
> One way to do that would be for nfsd to register a callback to be called
> from svc_delete_xprt(). Then in that callback nfsd can set those
> session flags appropriately, but it can also shut down the rpc client.
> That ensures the rpc client (and its xprt) go away before the svc_xprt,
> so we no longer need to hold a reference.
>
> (Actually there's nothing preventing multiple backchannels from sharing
> the same connection, so we'd need a list of callbacks.)
That would look like the below. The server would use it to register a
callback that cleans up the old rpc client, sets
SEQ4_STATUS_CB_PATH_DOWN if necessary, etc.
(Actually I think it may make the locking simplest just to keep these
callbacks under a spinlock, in which case the callback will need to
defer most of that work to a workqueue, which I don't see any problem
with so far.)
--b.
commit aecf73a3ef70426845deff43e4435a87ff9170ab
Author: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
Date: Mon Mar 22 15:37:17 2010 -0400
nfsd: provide callbacks on svc_xprt deletion.
NFSv4.1 needs warning when a client tcp connection goes down, if that
connection is being used as a backchannel, so that it can warn the
client that it has lost the backchannel connection.
This also allows us to destroy the rpc_client, or any other structures
associated with the backchannel connection, before the svc_xprt is
destroyed, thus removing the need for some odd reference counting.
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 5f4e18b..22229e4 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -32,6 +32,16 @@ struct svc_xprt_class {
u32 xcl_max_payload;
};
+/*
+ * This is embedded in an object that wants a callback before deleting
+ * an xprt; intended for use by NFSv4.1, which needs to know when a
+ * client's tcp connection (and hence possibly a backchannel) goes away.
+ */
+struct svc_xpt_user {
+ struct list_head list;
+ void (*callback)(struct svc_xpt_user *);
+};
+
struct svc_xprt {
struct svc_xprt_class *xpt_class;
struct svc_xprt_ops *xpt_ops;
@@ -66,8 +76,23 @@ struct svc_xprt {
struct sockaddr_storage xpt_remote; /* remote peer's address */
size_t xpt_remotelen; /* length of address */
struct rpc_wait_queue xpt_bc_pending; /* backchannel wait queue */
+ struct list_head xpt_users; /* callbacks on free */
};
+static inline void register_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u)
+{
+ spin_lock(&xpt->xpt_lock);
+ list_add(&u->list, &xpt->xpt_users);
+ spin_unlock(&xpt->xpt_lock);
+}
+
+static inline void unregister_xpt_user(struct svc_xprt *xpt, struct svc_xpt_user *u)
+{
+ spin_lock(&xpt->xpt_lock);
+ list_del_init(&u->list);
+ spin_unlock(&xpt->xpt_lock);
+}
+
int svc_reg_xprt_class(struct svc_xprt_class *);
void svc_unreg_xprt_class(struct svc_xprt_class *);
void svc_xprt_init(struct svc_xprt_class *, struct svc_xprt *,
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index c334f54..82cd43c 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -155,6 +155,7 @@ void svc_xprt_init(struct svc_xprt_class *xcl, struct svc_xprt *xprt,
INIT_LIST_HEAD(&xprt->xpt_list);
INIT_LIST_HEAD(&xprt->xpt_ready);
INIT_LIST_HEAD(&xprt->xpt_deferred);
+ INIT_LIST_HEAD(&xprt->xpt_users);
mutex_init(&xprt->xpt_mutex);
spin_lock_init(&xprt->xpt_lock);
set_bit(XPT_BUSY, &xprt->xpt_flags);
@@ -865,6 +866,18 @@ static void svc_age_temp_xprts(unsigned long closure)
mod_timer(&serv->sv_temptimer, jiffies + svc_conn_age_period * HZ);
}
+static void call_xpt_users(struct svc_xprt *xprt)
+{
+ struct svc_xpt_user *u;
+
+ spin_lock(&xprt->xpt_lock);
+ while (!list_empty(&xprt->xpt_users)) {
+ u = list_first_entry(&xprt->xpt_users, struct svc_xpt_user, list);
+ u->callback(u);
+ }
+ spin_unlock(&xprt->xpt_lock);
+}
+
/*
* Remove a dead transport
*/
@@ -897,6 +910,7 @@ void svc_delete_xprt(struct svc_xprt *xprt)
while ((dr = svc_deferred_dequeue(xprt)) != NULL)
kfree(dr);
+ call_xpt_users(xprt);
svc_xprt_put(xprt);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
