From patchwork Thu Apr 16 23:03:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 461866 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 746D2140293 for ; Fri, 17 Apr 2015 09:08:41 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="verification failed; unprotected key" header.d=gmail.com header.i=@gmail.com header.b=fvY/4c0b; dkim-adsp=none (unprotected policy); dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932087AbbDPXGe (ORCPT ); Thu, 16 Apr 2015 19:06:34 -0400 Received: from mail-qk0-f178.google.com ([209.85.220.178]:35144 "EHLO mail-qk0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753824AbbDPXEc (ORCPT ); Thu, 16 Apr 2015 19:04:32 -0400 Received: by qkhg7 with SMTP id g7so142780155qkh.2; Thu, 16 Apr 2015 16:04:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=EmWNtw4fzyzwGpM+pRZvsiOpHvzpzJbqzIOA7BkOAhc=; b=fvY/4c0bXrLwiUtNRfJcTd3m7WVT9SdLvNLXZ7xbeQKWC8oHBUIugICzFowETaoKql LfsoLqOn+37Hk3EOTq1lQzXGTSa0d3frbFsoBTcBbCAhkmJzqdx86gC/FKUL0ZsnPr11 R3MefyQIiTz+IrhbVi0gVorE9WLcEBdPWhSTIEP/xKlJmH6bLQuQwYLaPMnzqa5iokCI jN1+thUxV0J/Awi+0LKkyU5V5P7u1udkDYvYHKCzYF2uPXUrQvSbPTjmxeIqcbQMot27 RECzrGYCpLtfVZuVdDQzsvra2g/zQ4rIY38+ztOyfHHNUvQ3kcaj1cIreaVMb5ZHWV47 5T6g== X-Received: by 10.140.41.104 with SMTP id y95mr192728qgy.28.1429225471881; Thu, 16 Apr 2015 16:04:31 -0700 (PDT) Received: from htj.duckdns.org.lan (207-38-238-8.c3-0.wsd-ubr1.qens-wsd.ny.cable.rcn.com. [207.38.238.8]) by mx.google.com with ESMTPSA id z77sm6677670qkg.44.2015.04.16.16.04.30 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Apr 2015 16:04:31 -0700 (PDT) From: Tejun Heo To: akpm@linux-foundation.org, davem@davemloft.net Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Tejun Heo Subject: [PATCH 09/16] netconsole: replace target_list_lock with console_lock Date: Thu, 16 Apr 2015 19:03:46 -0400 Message-Id: <1429225433-11946-10-git-send-email-tj@kernel.org> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1429225433-11946-1-git-send-email-tj@kernel.org> References: <1429225433-11946-1-git-send-email-tj@kernel.org> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org netconsole has been using a spinlock - target_list_lock - to protect the list of configured netconsole targets and their enable/disable states. With the disabling from netdevice_notifier moved off to a workqueue by the previous patch and thus outside of rtnl_lock, target_list_lock can be replaced with console_lock, which allows us to avoid grabbing an extra lock in the log write path and can simplify locking when involving other subsystems as console_lock is only trylocked from printk path. This patch replaces target_list_lock with console_lock. The conversion is one-to-one except for write_msg(). The function is called with console_lock() already held so no further locking is necessary; however, as netpoll_send_udp() expects irq to be disabled, explicit irq save/restore pair is added around it. Signed-off-by: Tejun Heo Cc: David Miller --- drivers/net/netconsole.c | 50 ++++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index d355776..57c02ab 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -73,12 +73,12 @@ static int __init option_setup(char *opt) __setup("netconsole=", option_setup); #endif /* MODULE */ -/* Linked list of all configured targets */ +/* + * Linked list of all configured targets. The list and each target's + * enable/disable state are protected by console_lock. + */ static LIST_HEAD(target_list); -/* This needs to be a spinlock because write_msg() cannot sleep */ -static DEFINE_SPINLOCK(target_list_lock); - /** * struct netconsole_target - Represents a configured netconsole target. * @list: Links this target into the target_list. @@ -325,7 +325,6 @@ static ssize_t store_enabled(struct netconsole_target *nt, const char *buf, size_t count) { - unsigned long flags; int enabled; int err; @@ -357,9 +356,9 @@ static ssize_t store_enabled(struct netconsole_target *nt, * otherwise we might end up in write_msg() with * nt->np.dev == NULL and nt->enabled == true */ - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); nt->enabled = false; - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); netpoll_cleanup(&nt->np); } @@ -601,7 +600,6 @@ static struct config_item_type netconsole_target_type = { static struct config_item *make_netconsole_target(struct config_group *group, const char *name) { - unsigned long flags; struct netconsole_target *nt; nt = alloc_netconsole_target(); @@ -612,9 +610,9 @@ static struct config_item *make_netconsole_target(struct config_group *group, config_item_init_type_name(&nt->item, name, &netconsole_target_type); /* Adding, but it is disabled */ - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); list_add(&nt->list, &target_list); - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); return &nt->item; } @@ -622,12 +620,11 @@ static struct config_item *make_netconsole_target(struct config_group *group, static void drop_netconsole_target(struct config_group *group, struct config_item *item) { - unsigned long flags; struct netconsole_target *nt = to_target(item); - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); list_del(&nt->list); - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); /* * The target may have never been enabled, or was manually disabled @@ -664,11 +661,10 @@ static struct configfs_subsystem netconsole_subsys = { static void netconsole_deferred_disable_work_fn(struct work_struct *work) { struct netconsole_target *nt, *to_disable; - unsigned long flags; repeat: to_disable = NULL; - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); list_for_each_entry(nt, &target_list, list) { if (!nt->disable_scheduled) continue; @@ -682,7 +678,7 @@ repeat: to_disable = nt; break; } - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); if (to_disable) { netpoll_cleanup(&to_disable->np); @@ -698,7 +694,6 @@ static DECLARE_WORK(netconsole_deferred_disable_work, static int netconsole_netdev_event(struct notifier_block *this, unsigned long event, void *ptr) { - unsigned long flags; struct netconsole_target *nt; struct net_device *dev = netdev_notifier_info_to_dev(ptr); bool stopped = false; @@ -707,7 +702,7 @@ static int netconsole_netdev_event(struct notifier_block *this, event == NETDEV_RELEASE || event == NETDEV_JOIN)) goto done; - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); list_for_each_entry(nt, &target_list, list) { if (nt->np.dev == dev) { switch (event) { @@ -726,7 +721,7 @@ static int netconsole_netdev_event(struct notifier_block *this, } } } - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); if (stopped) { const char *msg = "had an event"; switch (event) { @@ -755,7 +750,6 @@ static struct notifier_block netconsole_netdev_notifier = { static void write_msg(struct console *con, const char *msg, unsigned int len) { int frag, left; - unsigned long flags; struct netconsole_target *nt; const char *tmp; @@ -765,9 +759,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len) if (list_empty(&target_list)) return; - spin_lock_irqsave(&target_list_lock, flags); list_for_each_entry(nt, &target_list, list) { - netconsole_target_get(nt); if (nt->enabled && netif_running(nt->np.dev)) { /* * We nest this inside the for-each-target loop above @@ -783,9 +775,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len) left -= frag; } } - netconsole_target_put(nt); } - spin_unlock_irqrestore(&target_list_lock, flags); } static struct console netconsole = { @@ -798,7 +788,6 @@ static int __init init_netconsole(void) { int err; struct netconsole_target *nt, *tmp; - unsigned long flags; char *target_config; char *input = config; @@ -812,9 +801,9 @@ static int __init init_netconsole(void) /* Dump existing printks when we register */ netconsole.flags |= CON_PRINTBUFFER; - spin_lock_irqsave(&target_list_lock, flags); + console_lock(); list_add(&nt->list, &target_list); - spin_unlock_irqrestore(&target_list_lock, flags); + console_unlock(); } } @@ -839,8 +828,8 @@ fail: /* * Remove all targets and destroy them (only targets created - * from the boot/module option exist here). Skipping the list - * lock is safe here, and netpoll_cleanup() will sleep. + * from the boot/module option exist here). Skipping the console + * lock is safe here. */ list_for_each_entry_safe(nt, tmp, &target_list, list) { list_del(&nt->list); @@ -864,8 +853,7 @@ static void __exit cleanup_netconsole(void) * and would first be rmdir(2)'ed from userspace. We reach * here only when they are already destroyed, and only those * created from the boot/module option are left, so remove and - * destroy them. Skipping the list lock is safe here, and - * netpoll_cleanup() will sleep. + * destroy them. Skipping the console lock is safe here. */ list_for_each_entry_safe(nt, tmp, &target_list, list) { list_del(&nt->list);