From patchwork Fri Mar 11 15:29:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 8566481 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 197DE9F7CA for ; Fri, 11 Mar 2016 15:29:35 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3156820272 for ; Fri, 11 Mar 2016 15:29:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E748C20328 for ; Fri, 11 Mar 2016 15:29:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932579AbcCKP33 (ORCPT ); Fri, 11 Mar 2016 10:29:29 -0500 Received: from www.linutronix.de ([62.245.132.108]:37923 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932567AbcCKP3Z (ORCPT ); Fri, 11 Mar 2016 10:29:25 -0500 Received: from localhost ([127.0.0.1] helo=bazinga.breakpoint.cc) by Galois.linutronix.de with esmtp (Exim 4.80) (envelope-from ) id 1aeP0G-0006Yk-09; Fri, 11 Mar 2016 16:29:24 +0100 From: Sebastian Andrzej Siewior To: linux-scsi@vger.kernel.org Cc: "James E.J. Bottomley" , "Martin K. Petersen" , rt@linutronix.de, Sebastian Andrzej Siewior , QLogic-Storage-Upstream@qlogic.com, Christoph Hellwig Subject: [PATCH 10/11] scsi: bnx2fc: fix hotplug race in bnx2fc_process_new_cqes() Date: Fri, 11 Mar 2016 16:29:02 +0100 Message-Id: <1457710143-29182-11-git-send-email-bigeasy@linutronix.de> X-Mailer: git-send-email 2.7.0 In-Reply-To: <1457710143-29182-1-git-send-email-bigeasy@linutronix.de> References: <1457710143-29182-1-git-send-email-bigeasy@linutronix.de> X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1, SHORTCIRCUIT=-0.0001, URIBL_BLOCKED=0.001 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 The ->iothread is accessed without holding the lock. Take this: CPU A CPU B ------- ------- bnx2fc_process_new_cqes() bnx2fc_percpu_thread_destroy() spin_lock_bh(fp_work_lock); fps->iothread != NULL list_add_tail(work) spin_unlock_bh(&fps->fp_work_lock); spin_lock_bh(&fps->fp_work_lock); fps->iothread = NULL if (fps->iothread && work) ... else bnx2fc_process_cq_compl(work) bnx2fc_process_cq_compl(work); CPU A will process wqe despite having it added to the work list of CPU B which will at the same time clean up the queued wqe. The fix is to add the item to the list and wakeup the thread while still holding the lock. If the item was not added to the list then the `process' variable is still true in which case we have to do manually. Cc: QLogic-Storage-Upstream@qlogic.com Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Christoph Hellwig Cc: linux-scsi@vger.kernel.org Signed-off-by: Sebastian Andrzej Siewior --- drivers/scsi/bnx2fc/bnx2fc_hwi.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c b/drivers/scsi/bnx2fc/bnx2fc_hwi.c index 28c671b609b2..1427062e86f0 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c @@ -1045,6 +1045,7 @@ int bnx2fc_process_new_cqes(struct bnx2fc_rport *tgt) struct bnx2fc_work *work = NULL; struct bnx2fc_percpu_s *fps = NULL; unsigned int cpu = wqe % num_possible_cpus(); + bool process = true; fps = &per_cpu(bnx2fc_percpu, cpu); spin_lock_bh(&fps->fp_work_lock); @@ -1052,16 +1053,16 @@ int bnx2fc_process_new_cqes(struct bnx2fc_rport *tgt) goto unlock; work = bnx2fc_alloc_work(tgt, wqe); - if (work) + if (work) { list_add_tail(&work->list, &fps->work_list); + wake_up_process(fps->iothread); + process = false; + } unlock: spin_unlock_bh(&fps->fp_work_lock); - /* Pending work request completion */ - if (fps->iothread && work) - wake_up_process(fps->iothread); - else + if (process) bnx2fc_process_cq_compl(tgt, wqe); num_free_sqes++; }