From patchwork Wed May 17 15:01:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 9731395 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 646E060363 for ; Wed, 17 May 2017 15:02:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5712A28798 for ; Wed, 17 May 2017 15:02:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4B7DD287B2; Wed, 17 May 2017 15:02:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ADDFE28798 for ; Wed, 17 May 2017 15:02:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753679AbdEQPCE (ORCPT ); Wed, 17 May 2017 11:02:04 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:46377 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751398AbdEQPCE (ORCPT ); Wed, 17 May 2017 11:02:04 -0400 Received: from bigeasy by Galois.linutronix.de with local (Exim 4.80) (envelope-from ) id 1dB0Rw-0004Ba-1t; Wed, 17 May 2017 17:01:16 +0200 Date: Wed, 17 May 2017 17:01:53 +0200 From: Sebastian Andrzej Siewior To: Chad Dupuis Cc: James Bottomley , "Martin K. Petersen" , linux-scsi@vger.kernel.org, Chris Leech , Chad Dupuis , rt@linutronix.de, Lee Duncan , QLogic-Storage-Upstream@qlogic.com, Andrew Morton , Johannes Thumshirn , Christoph Hellwig Subject: Re: [REEEEPOST] bnx2i + bnx2fc: convert to generic workqueue (#3) Message-ID: <20170517150153.2eamcjtc4frx2cae@linutronix.de> References: <20170410171254.30367-1-bigeasy@linutronix.de> <20170504174427.6hebbnqwfgems6dg@linutronix.de> <1494343100.2688.34.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170306 (1.8.0) Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 2017-05-12 11:55:52 [-0400], Chad Dupuis wrote: > Ok, I believe I've found the issue here. The machine that the test has > performed on had many more possible CPUs than active CPUs. We calculate > which CPU to the work time on in bnx2fc_process_new_cqes() like this: > > unsigned int cpu = wqe % num_possible_cpus(); > > Since not all CPUs are active, we were trying to schedule work on > non-active CPUs which meant that the upper layers were never notified of > the completion. With this change: > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c > b/drivers/scsi/bnx2fc/bnx2fc_hwi.c > index c2288d6..6f08e43 100644 > --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c > +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c > @@ -1042,7 +1042,12 @@ static int bnx2fc_process_new_cqes(struct > bnx2fc_rport *tgt) > /* Pending work request completion */ > struct bnx2fc_work *work = NULL; > struct bnx2fc_percpu_s *fps = NULL; > - unsigned int cpu = wqe % num_possible_cpus(); > + unsigned int cpu = wqe % num_active_cpus(); > + > + /* Sanity check cpu to make sure it's online */ > + if (!cpu_active(cpu)) > + /* Default to CPU 0 */ > + cpu = 0; > > work = bnx2fc_alloc_work(tgt, wqe); > if (work) { > > The issue is fixed. > > Sebastian, can you add this change to your patch set? Are sure that you can reliably reproduce the issue and fix it with the patch above? Because this patch: produces this output: [ 1.960313] Unregister pv shared memory for cpu 3 [ 1.997000] kernel_init_freeable() num possible: 8 [ 1.998073] kernel_init_freeable() num online : 7 [ 1.999125] kernel_init_freeable() 3 active : 0 [ 2.000337] workfn() 1 which means, CPU3 is offline and work runs on CPU1 instead. So it does already what you suggest except that chances are, that it is not run on CPU0 in this case (but on another CPU). So it either takes some time for wait_for_completion(&io_req->tm_done); to come back _or_ there is a leak somewhere where a complete() is somehow missing / racing against something. Sebastian diff --git a/init/main.c b/init/main.c index b0c11cbf5ddf..483a971b1fd2 100644 --- a/init/main.c +++ b/init/main.c @@ -997,6 +997,12 @@ static int __ref kernel_init(void *unused) "See Linux Documentation/admin-guide/init.rst for guidance."); } +static void workfn(struct work_struct *work) +{ + pr_err("%s() %d\n", __func__, raw_smp_processor_id()); +} +static DECLARE_WORK(work, workfn); + static noinline void __init kernel_init_freeable(void) { /* @@ -1040,6 +1046,15 @@ static noinline void __init kernel_init_freeable(void) (void) sys_dup(0); (void) sys_dup(0); + { + + cpu_down(3); + pr_err("%s() num possible: %d\n", __func__, num_possible_cpus()); + pr_err("%s() num online : %d\n", __func__, num_online_cpus()); + pr_err("%s() 3 active : %d\n", __func__, cpu_active(3)); + schedule_work_on(3, &work); + ssleep(5); + } /* * check if there is an early userspace init. If yes, let it do all * the work