From patchwork Tue Oct 28 16:34:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sagi Grimberg X-Patchwork-Id: 5178311 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CF6CD9F387 for ; Tue, 28 Oct 2014 16:35:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9E0A2201D3 for ; Tue, 28 Oct 2014 16:35:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A634520179 for ; Tue, 28 Oct 2014 16:35:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751547AbaJ1QfH (ORCPT ); Tue, 28 Oct 2014 12:35:07 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:57279 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751005AbaJ1QfG (ORCPT ); Tue, 28 Oct 2014 12:35:06 -0400 Received: by mail-wi0-f173.google.com with SMTP id ex7so9895450wid.6 for ; Tue, 28 Oct 2014 09:35:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=reqmCkuUPCBQLOp8eEB9Dv0xLR/b5Zu9pBRKhaojcEo=; b=S22TUNNBr0ap3l3lLxb7WLxOoKDbyX4mns9lbEndAf2QJlP3j/HI+w2K8tWJkyfyyI rXpWvdZ2BBeBu4gOB1kor/iWMZU5jdeqClm9AI2Zz/FDZbuU7Yk95QPMeYucl6Ec8J5/ Bm2phC4EDPzGPJTnGdpMdJ6Jb8WzDkMi8xyN4cLJkXz76ODiN2Kzg3Nma0kHrPn/yTEh QeTOfpXQEQRYYbti+JjjOpXxMYnvxZ9223Nwz0bs8eCsIx9PjYY0uaCJgvfP/hEjE/m5 IEu7rjAB3ByjFjNvXTS5i2j/MT8fxqq2lCAL+KBhy6iVmFnQvbp+V9J5Rm4fGMUiHi1v 2N3Q== X-Gm-Message-State: ALoCoQlufsW9DDqWNjvA3LS9vVP+IB7XxbGo5538qkxraB4LQ+by62pvTf3LVq84yGqZZBLcWhpV X-Received: by 10.180.211.166 with SMTP id nd6mr5979672wic.81.1414514103953; Tue, 28 Oct 2014 09:35:03 -0700 (PDT) Received: from [172.25.5.3] ([193.47.165.251]) by mx.google.com with ESMTPSA id o1sm2384483wja.25.2014.10.28.09.35.02 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 28 Oct 2014 09:35:03 -0700 (PDT) Message-ID: <544FC5B2.70104@dev.mellanox.co.il> Date: Tue, 28 Oct 2014 18:34:58 +0200 From: Sagi Grimberg User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: "Nicholas A. Bellinger" , Or Gerlitz CC: target-devel , linux-rdma Subject: Re: ib_isert RDMA_CM_EVENT_DEVICE_REMOVAL events References: <1414130562.15076.27.camel@haakon3.risingtidesystems.com> In-Reply-To: <1414130562.15076.27.camel@haakon3.risingtidesystems.com> Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 10/24/2014 9:02 AM, Nicholas A. Bellinger wrote: > AFAICT, it looks like the assumption in isert_disconnected_handler() to > dereference rdma_cm_id->context as isert_conn (in all cases) is wrong, > and the above RDMA_CM_EVENT_DEVICE_REMOVAL has iscsi_np stored in > ->context from the original rdma_create_id() at isert_setup_np() time. > > So, is there a way to tell the difference how rdma_cm_id->context should > be dereferenced when DEVICE_REMOVAL occurs..? Does DEVICE_REMOVAL occur > on just the listener rdma_cm_id, or on all accepted children as well..? > > Anything else to consider wrt to other CMA events being kicked off into > isert_disconnected_handler()..? > Hey Nic, Terribly sorry for the late response, I'm juggling between 5 different projects... This is indeed a bug, and I indeed noticed it. This will happen if the network portal cm id listens on a specific address (e.g. not any - 0.0.0.0), in this case the cm id will acquire the relevant device (see rdma_bind_addr) - hence will sense DEVICE_REMOVAL events. And yes, all the accepted children will of course get the event as well. Notice that cma_remove_one sequence (that fires DEVICE_REMOVAL event to all the relevant cma ids) requires the cmd is owner to finish cleanup before the end of the callback because in the end of it, it will allow the device to remove. So I do plan to get disconnected_handler to the callback instead of the deferred work. I think this should make the bug you hit go away: iser-target: Handle DEVICE_REMOVAL event on network portal listener correctly In this case the cm_id->context is the isert_np, and the cm_id->qp is NULL, so use that to distinct the cases. Since we don't expect any other events on this cm_id we can just return -1 for explicit termination of the cm_id by the cma layer. Signed-off-by: Sagi Grimberg --- drivers/infiniband/ulp/isert/ib_isert.c | 29 +++++++++++++++++++---------- 1 files changed, 19 insertions(+), 10 deletions(-) static int @@ -825,6 +836,9 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event) switch (event->event) { case RDMA_CM_EVENT_CONNECT_REQUEST: ret = isert_connect_request(cma_id, event); + if (ret) + pr_err("isert_cma_handler failed RDMA_CM_EVENT: 0x%08x %d\n", + event->event, ret); break; case RDMA_CM_EVENT_ESTABLISHED: isert_connected_handler(cma_id); @@ -834,7 +848,7 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event) case RDMA_CM_EVENT_DEVICE_REMOVAL: /* FALLTHRU */ disconnect = true; case RDMA_CM_EVENT_TIMEWAIT_EXIT: /* FALLTHRU */ - isert_disconnected_handler(cma_id, disconnect); + ret = isert_disconnected_handler(cma_id, disconnect); break; case RDMA_CM_EVENT_CONNECT_ERROR: default: @@ -842,12 +856,6 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event) break; } - if (ret != 0) { - pr_err("isert_cma_handler failed RDMA_CM_EVENT: 0x%08x %d\n", - event->event, ret); - dump_stack(); - } - return ret; } @@ -3203,7 +3211,8 @@ isert_free_np(struct iscsi_np *np) { struct isert_np *isert_np = (struct isert_np *)np->np_context; - rdma_destroy_id(isert_np->np_cm_id); + if (isert_np->np_cm_id) + rdma_destroy_id(isert_np->np_cm_id); np->np_context = NULL; kfree(isert_np); -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c index 5552f93..d620d5e 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.c +++ b/drivers/infiniband/ulp/isert/ib_isert.c @@ -803,14 +803,25 @@ wake_up: complete(&isert_conn->conn_wait); } -static void +static int isert_disconnected_handler(struct rdma_cm_id *cma_id, bool disconnect) { - struct isert_conn *isert_conn = (struct isert_conn *)cma_id->context; + struct isert_conn *isert_conn; + + if (!cma_id->qp) { + struct isert_np *isert_np = cma_id->context; + + isert_np->np_cm_id = NULL; + return -1; + } + + isert_conn = (struct isert_conn *)cma_id->context; isert_conn->disconnect = disconnect; INIT_WORK(&isert_conn->conn_logout_work, isert_disconnect_work); schedule_work(&isert_conn->conn_logout_work); + + return 0; }