From patchwork Mon Apr 10 17:12:51 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: 9673657 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 0325C60244 for ; Mon, 10 Apr 2017 17:13:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DE13126247 for ; Mon, 10 Apr 2017 17:13:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D2B4026E76; Mon, 10 Apr 2017 17:13:20 +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 3EAFB26247 for ; Mon, 10 Apr 2017 17:13:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754060AbdDJRNT (ORCPT ); Mon, 10 Apr 2017 13:13:19 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:52163 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755678AbdDJRNR (ORCPT ); Mon, 10 Apr 2017 13:13:17 -0400 Received: from localhost ([127.0.0.1] helo=bazinga.breakpoint.cc) by Galois.linutronix.de with esmtp (Exim 4.80) (envelope-from ) id 1cxcr9-00015Z-Sa; Mon, 10 Apr 2017 19:12:00 +0200 From: Sebastian Andrzej Siewior To: "Martin K . Petersen" , "James E.J. Bottomley" , linux-scsi@vger.kernel.org Cc: rt@linutronix.de, Lee Duncan , Chris Leech , Chad Dupuis , QLogic-Storage-Upstream@qlogic.com, Johannes Thumshirn , Christoph Hellwig , Andrew Morton , Sebastian Andrzej Siewior Subject: [PATCH 2/5] scsi: bnx2fc: convert per-CPU thread to workqueue Date: Mon, 10 Apr 2017 19:12:51 +0200 Message-Id: <20170410171254.30367-3-bigeasy@linutronix.de> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170410171254.30367-1-bigeasy@linutronix.de> References: <20170410171254.30367-1-bigeasy@linutronix.de> MIME-Version: 1.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 The driver creates its own per-CPU threads which are updated based on CPU hotplug events. It is also possible to use kworkers and remove some of the infrastructure get the same job done while saving a few lines of code. bnx2fc_percpu_io_thread() becomes bnx2fc_percpu_io_work() which is mostly the same code. The outer loop (kthread_should_stop()) gets removed and the remaining code is shifted to the left. In bnx2fc_process_new_cqes() the code checked for ->iothread to decide if there is an active per-CPU thread. With the kworkers this is no longer possible nor required. The allocation of a new work item (via bnx2fc_alloc_work()) does no longer happen with the ->fp_work_lock lock held. It performs only a memory allocation + initialization which does not require any kind of serialization. The lock is held while adding the new member to fps->work_list list. The remaining part is the removal CPU hotplug notifier since it is taken care by the workqueue code. This patch was only compile-tested due to -ENODEV. Cc: QLogic-Storage-Upstream@qlogic.com Cc: Christoph Hellwig Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Christoph Hellwig Reviewed-by: Johannes Thumshirn --- drivers/scsi/bnx2fc/bnx2fc.h | 2 +- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 144 ++++++-------------------------------- drivers/scsi/bnx2fc/bnx2fc_hwi.c | 22 +++--- 3 files changed, 33 insertions(+), 135 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h index 4fc8ed5fe067..0279cc8de7a0 100644 --- a/drivers/scsi/bnx2fc/bnx2fc.h +++ b/drivers/scsi/bnx2fc/bnx2fc.h @@ -168,7 +168,7 @@ extern struct fcoe_percpu_s bnx2fc_global; extern struct workqueue_struct *bnx2fc_wq; struct bnx2fc_percpu_s { - struct task_struct *iothread; + struct work_struct work; struct list_head work_list; spinlock_t fp_work_lock; }; diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 93b5a0012417..329922d51f8a 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -614,39 +614,32 @@ static void bnx2fc_recv_frame(struct sk_buff *skb) } /** - * bnx2fc_percpu_io_thread - thread per cpu for ios + * bnx2fc_percpu_io_work - work per cpu for ios * - * @arg: ptr to bnx2fc_percpu_info structure + * @work_s: The work struct */ -static int bnx2fc_percpu_io_thread(void *arg) +static void bnx2fc_percpu_io_work(struct work_struct *work_s) { - struct bnx2fc_percpu_s *p = arg; + struct bnx2fc_percpu_s *p; struct bnx2fc_work *work, *tmp; LIST_HEAD(work_list); - set_user_nice(current, MIN_NICE); - set_current_state(TASK_INTERRUPTIBLE); - while (!kthread_should_stop()) { - schedule(); - spin_lock_bh(&p->fp_work_lock); - while (!list_empty(&p->work_list)) { - list_splice_init(&p->work_list, &work_list); - spin_unlock_bh(&p->fp_work_lock); + p = container_of(work_s, struct bnx2fc_percpu_s, work); - list_for_each_entry_safe(work, tmp, &work_list, list) { - list_del_init(&work->list); - bnx2fc_process_cq_compl(work->tgt, work->wqe); - kfree(work); - } - - spin_lock_bh(&p->fp_work_lock); - } - __set_current_state(TASK_INTERRUPTIBLE); + spin_lock_bh(&p->fp_work_lock); + while (!list_empty(&p->work_list)) { + list_splice_init(&p->work_list, &work_list); spin_unlock_bh(&p->fp_work_lock); - } - __set_current_state(TASK_RUNNING); - return 0; + list_for_each_entry_safe(work, tmp, &work_list, list) { + list_del_init(&work->list); + bnx2fc_process_cq_compl(work->tgt, work->wqe); + kfree(work); + } + + spin_lock_bh(&p->fp_work_lock); + } + spin_unlock_bh(&p->fp_work_lock); } static struct fc_host_statistics *bnx2fc_get_host_stats(struct Scsi_Host *shost) @@ -2563,73 +2556,6 @@ static struct fcoe_transport bnx2fc_transport = { .disable = bnx2fc_disable, }; -/** - * bnx2fc_percpu_thread_create - Create a receive thread for an - * online CPU - * - * @cpu: cpu index for the online cpu - */ -static void bnx2fc_percpu_thread_create(unsigned int cpu) -{ - struct bnx2fc_percpu_s *p; - struct task_struct *thread; - - p = &per_cpu(bnx2fc_percpu, cpu); - - thread = kthread_create_on_node(bnx2fc_percpu_io_thread, - (void *)p, cpu_to_node(cpu), - "bnx2fc_thread/%d", cpu); - /* bind thread to the cpu */ - if (likely(!IS_ERR(thread))) { - kthread_bind(thread, cpu); - p->iothread = thread; - wake_up_process(thread); - } -} - -static void bnx2fc_percpu_thread_destroy(unsigned int cpu) -{ - struct bnx2fc_percpu_s *p; - struct task_struct *thread; - struct bnx2fc_work *work, *tmp; - - BNX2FC_MISC_DBG("destroying io thread for CPU %d\n", cpu); - - /* Prevent any new work from being queued for this CPU */ - p = &per_cpu(bnx2fc_percpu, cpu); - spin_lock_bh(&p->fp_work_lock); - thread = p->iothread; - p->iothread = NULL; - - - /* Free all work in the list */ - list_for_each_entry_safe(work, tmp, &p->work_list, list) { - list_del_init(&work->list); - bnx2fc_process_cq_compl(work->tgt, work->wqe); - kfree(work); - } - - spin_unlock_bh(&p->fp_work_lock); - - if (thread) - kthread_stop(thread); -} - - -static int bnx2fc_cpu_online(unsigned int cpu) -{ - printk(PFX "CPU %x online: Create Rx thread\n", cpu); - bnx2fc_percpu_thread_create(cpu); - return 0; -} - -static int bnx2fc_cpu_dead(unsigned int cpu) -{ - printk(PFX "CPU %x offline: Remove Rx thread\n", cpu); - bnx2fc_percpu_thread_destroy(cpu); - return 0; -} - static int bnx2fc_slave_configure(struct scsi_device *sdev) { if (!bnx2fc_queue_depth) @@ -2699,33 +2625,13 @@ static int __init bnx2fc_mod_init(void) p = &per_cpu(bnx2fc_percpu, cpu); INIT_LIST_HEAD(&p->work_list); spin_lock_init(&p->fp_work_lock); + INIT_WORK(&p->work, bnx2fc_percpu_io_work); } - get_online_cpus(); - - for_each_online_cpu(cpu) - bnx2fc_percpu_thread_create(cpu); - - rc = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, - "scsi/bnx2fc:online", - bnx2fc_cpu_online, NULL); - if (rc < 0) - goto stop_threads; - bnx2fc_online_state = rc; - - cpuhp_setup_state_nocalls(CPUHP_SCSI_BNX2FC_DEAD, "scsi/bnx2fc:dead", - NULL, bnx2fc_cpu_dead); - put_online_cpus(); - cnic_register_driver(CNIC_ULP_FCOE, &bnx2fc_cnic_cb); return 0; -stop_threads: - for_each_online_cpu(cpu) - bnx2fc_percpu_thread_destroy(cpu); - put_online_cpus(); - kthread_stop(l2_thread); free_wq: destroy_workqueue(bnx2fc_wq); release_bt: @@ -2784,17 +2690,13 @@ static void __exit bnx2fc_mod_exit(void) if (l2_thread) kthread_stop(l2_thread); - get_online_cpus(); - /* Destroy per cpu threads */ - for_each_online_cpu(cpu) { - bnx2fc_percpu_thread_destroy(cpu); + for_each_possible_cpu(cpu) { + struct bnx2fc_percpu_s *p; + + p = per_cpu_ptr(&bnx2fc_percpu, cpu); + flush_work(&p->work); } - cpuhp_remove_state_nocalls(bnx2fc_online_state); - cpuhp_remove_state_nocalls(CPUHP_SCSI_BNX2FC_DEAD); - - put_online_cpus(); - destroy_workqueue(bnx2fc_wq); /* * detach from scsi transport diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c b/drivers/scsi/bnx2fc/bnx2fc_hwi.c index 5ff9f89c17c7..1ed7a1784e15 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c @@ -1046,23 +1046,19 @@ int bnx2fc_process_new_cqes(struct bnx2fc_rport *tgt) struct bnx2fc_percpu_s *fps = NULL; unsigned int cpu = wqe % num_possible_cpus(); - fps = &per_cpu(bnx2fc_percpu, cpu); - spin_lock_bh(&fps->fp_work_lock); - if (unlikely(!fps->iothread)) - goto unlock; - work = bnx2fc_alloc_work(tgt, wqe); - if (work) + if (work) { + fps = &per_cpu(bnx2fc_percpu, cpu); + + spin_lock_bh(&fps->fp_work_lock); list_add_tail(&work->list, &fps->work_list); -unlock: - spin_unlock_bh(&fps->fp_work_lock); - - /* Pending work request completion */ - if (fps->iothread && work) - wake_up_process(fps->iothread); - else + spin_unlock_bh(&fps->fp_work_lock); + schedule_work_on(cpu, &fps->work); + } else { bnx2fc_process_cq_compl(tgt, wqe); + } + num_free_sqes++; } cqe++;