Skip to content

Commit

Permalink
libceph: separate non-locked fault handling
Browse files Browse the repository at this point in the history
An error occurring on a ceph connection is treated as a fault,
causing the connection to be reset.  The initial part of this fault
handling has to be done while holding the connection mutex, but
it must then be dropped for the last part.

Separate the part of this fault handling that executes without the
lock into its own function, con_fault_finish().  Move the call to
this new function, as well as call that drops the connection mutex,
into ceph_fault().  Rename that function con_fault() to reflect that
it's only handling the connection part of the fault handling.

The motivation for this was a warning from sparse about the locking
being done here.  Rearranging things this way keeps all the mutex
manipulation within ceph_fault(), and this stops sparse from
complaining.

This partially resolves:
    http://tracker.ceph.com/issues/4184

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
  • Loading branch information
Alex Elder committed Feb 25, 2013
1 parent f20a39f commit 9320926
Showing 1 changed file with 23 additions and 19 deletions.
42 changes: 23 additions & 19 deletions net/ceph/messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ static struct lock_class_key socket_class;

static void queue_con(struct ceph_connection *con);
static void con_work(struct work_struct *);
static void ceph_fault(struct ceph_connection *con);
static void con_fault(struct ceph_connection *con);

/*
* Nicely render a sockaddr as a string. An array of formatted
Expand Down Expand Up @@ -2363,6 +2363,23 @@ static bool con_backoff(struct ceph_connection *con)
return true;
}

/* Finish fault handling; con->mutex must *not* be held here */

static void con_fault_finish(struct ceph_connection *con)
{
/*
* in case we faulted due to authentication, invalidate our
* current tickets so that we can get new ones.
*/
if (con->auth_retry && con->ops->invalidate_authorizer) {
dout("calling invalidate_authorizer()\n");
con->ops->invalidate_authorizer(con);
}

if (con->ops->fault)
con->ops->fault(con);
}

/*
* Do some work on a connection. Drop a connection ref when we're done.
*/
Expand Down Expand Up @@ -2419,7 +2436,9 @@ static void con_work(struct work_struct *work)
return;

fault:
ceph_fault(con); /* error/fault path */
con_fault(con);
mutex_unlock(&con->mutex);
con_fault_finish(con);
goto done_unlocked;
}

Expand All @@ -2428,8 +2447,7 @@ static void con_work(struct work_struct *work)
* Generic error/fault handler. A retry mechanism is used with
* exponential backoff
*/
static void ceph_fault(struct ceph_connection *con)
__releases(con->mutex)
static void con_fault(struct ceph_connection *con)
{
pr_warning("%s%lld %s %s\n", ENTITY_NAME(con->peer_name),
ceph_pr_addr(&con->peer_addr.in_addr), con->error_msg);
Expand All @@ -2445,7 +2463,7 @@ static void ceph_fault(struct ceph_connection *con)
if (con_flag_test(con, CON_FLAG_LOSSYTX)) {
dout("fault on LOSSYTX channel, marking CLOSED\n");
con->state = CON_STATE_CLOSED;
goto out_unlock;
return;
}

if (con->in_msg) {
Expand Down Expand Up @@ -2476,20 +2494,6 @@ static void ceph_fault(struct ceph_connection *con)
con_flag_set(con, CON_FLAG_BACKOFF);
queue_con(con);
}

out_unlock:
mutex_unlock(&con->mutex);
/*
* in case we faulted due to authentication, invalidate our
* current tickets so that we can get new ones.
*/
if (con->auth_retry && con->ops->invalidate_authorizer) {
dout("calling invalidate_authorizer()\n");
con->ops->invalidate_authorizer(con);
}

if (con->ops->fault)
con->ops->fault(con);
}


Expand Down

0 comments on commit 9320926

Please sign in to comment.