From patchwork Fri Feb 22 17:26:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 2176711 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 5F2EE3FD4E for ; Fri, 22 Feb 2013 17:26:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758143Ab3BVR0J (ORCPT ); Fri, 22 Feb 2013 12:26:09 -0500 Received: from mail-ie0-f170.google.com ([209.85.223.170]:40971 "EHLO mail-ie0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758035Ab3BVR0H (ORCPT ); Fri, 22 Feb 2013 12:26:07 -0500 Received: by mail-ie0-f170.google.com with SMTP id c11so1003845ieb.1 for ; Fri, 22 Feb 2013 09:26:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=PRPzOUnAhErOhhFvACxjiXRoHBg8NyicLrZpU+Zp5C4=; b=HcoHnWfo/VhGbIaZOlUVW0B+Xkrh01KrbmA0oxvSPwhwwq+9kKp+CoC0z3i/FSgvOW HZ4UfFAtL/cDdlK7pH50WJbraS6eWNRGVSzaFVi5crRmEUUyWwoAxzXDv5O/ewQbzovO LVz86SrHWKfQHzY/RMJ0oX5Fh2yez6ZllFdQuQ68gsEz0fAz/62ij46K8pYNhiBvryby OVnLk+gBpTZHV4hqi1jMX0WicDlQC1rbDsi4ufXFsNZgR/7Yeee1YXtJbcMGyPUYDldc K8grwUJ3hWeoa2TKd8VUQRRBlPZmSrgEY2Im+AwgCwUeMDOmJdDmELi76MhAnml5619l UJEw== X-Received: by 10.50.45.200 with SMTP id p8mr15197053igm.104.1361553966831; Fri, 22 Feb 2013 09:26:06 -0800 (PST) Received: from [172.22.22.4] (c-71-195-31-37.hsd1.mn.comcast.net. [71.195.31.37]) by mx.google.com with ESMTPS id a3sm2075691igq.5.2013.02.22.09.26.05 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 22 Feb 2013 09:26:05 -0800 (PST) Message-ID: <5127AA2C.7090709@inktank.com> Date: Fri, 22 Feb 2013 11:26:04 -0600 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-Version: 1.0 To: "ceph-devel@vger.kernel.org >> ceph-devel" Subject: [PATCH 2/5] libceph: separate non-locked fault handling References: <5127A85D.1070000@inktank.com> <5127A935.8020605@inktank.com> In-Reply-To: <5127A935.8020605@inktank.com> X-Gm-Message-State: ALoCoQmYgRSG9kOYjPkfD5Za+HYukzvKJ2rea/WXgg3fDnrTWLFOsM0D+tQDvJ5VfsCwlqnbYlS8 Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org 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 Signed-off-by: Alex Elder --- v2: rebased net/ceph/messenger.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 9a29d8a..c3b9060 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -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 @@ -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. */ @@ -2419,7 +2436,9 @@ done_unlocked: return; fault: - ceph_fault(con); /* error/fault path */ + con_fault(con); + mutex_unlock(&con->mutex); + con_fault_finish(con); goto done_unlocked; } @@ -2428,8 +2447,7 @@ fault: * 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); @@ -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) { @@ -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); }