From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxim Uvarov Subject: Re: [PATCH] bond_alb: don't disable softirq under bond_alb_xmit Date: Mon, 09 Jan 2012 10:59:47 -0800 Message-ID: <4F0B3923.4020707@oracle.com> References: <1325881423-19309-1-git-send-email-maxim.uvarov@oracle.com> <7449.1325885605@death> <20120107.101450.2214394343146869929.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: fubar@us.ibm.com, netdev@vger.kernel.org, andy@greyhouse.net, amwang@redhat.com To: David Miller Return-path: Received: from rcsinet15.oracle.com ([148.87.113.117]:20425 "EHLO rcsinet15.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932546Ab2AITCW (ORCPT ); Mon, 9 Jan 2012 14:02:22 -0500 In-Reply-To: <20120107.101450.2214394343146869929.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: On 01/07/2012 10:14 AM, David Miller wrote: > From: Jay Vosburgh > Date: Fri, 06 Jan 2012 13:33:25 -0800 > >> Maxim Uvarov wrote: >> >>> No need to lock soft irqs under bond_alb_xmit() >>> which already has softirq disabled. >> >> In commit: >> >> commit 6603a6f25e4bca922a7dfbf0bf03072d98850176 >> Author: Jay Vosburgh >> Date: Wed Oct 17 17:37:50 2007 -0700 >> >> bonding: Convert more locks to _bh, acquire rtnl, for new locking >> >> Convert more lock acquisitions to _bh flavor to avoid deadlock >> with workqueue activity and add acquisition of RTNL in appropriate places. >> Affects ALB mode, as well as core bonding functions and sysfs. >> >> Signed-off-by: Andy Gospodarek >> Signed-off-by: Jay Vosburgh >> Signed-off-by: Jeff Garzik >> >> the _lock_tx_hashtbl was upgraded from regular to _bh to prevent >> deadlocks. I don't recall right offhand what deadlock this prevented, >> but are we sure there are no possible issues with converting this lock >> back to a non-_bh acquisition? > > Maxim's patch is not changing the BH'ness of the list. > > > He's just avoiding a BH disable which is unnecessary because BH is > already disabled in the effected code path(s). > Yes, I only removed disabling BH for tlb_choose_channel(). In other places this lock still disables BH. This makes lock more accurate, because there are 2 paths for execution: 1. dev_queue_xmit() and BH are already disabled. 2. netpoll and irqs are disabled. So no need to enable/disable BH. This patch also removes waring: WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0x69/0x80() [] ? local_bh_enable_ip+0x69/0x80 [] warn_slowpath_common+0x81/0xa0 [] ? local_bh_enable_ip+0x69/0x80 [] warn_slowpath_null+0x22/0x30 [] local_bh_enable_ip+0x69/0x80 [] _raw_spin_unlock_bh+0x13/0x20 [] tlb_choose_channel+0x50/0xb0 [bonding] [] bond_alb_xmit+0x207/0x210 [bonding] [] __bond_start_xmit+0x177/0x1a0 [bonding] [] bond_start_xmit+0x46/0x80 [bonding] [] netpoll_send_skb_on_dev+0x121/0x1a0 [] ? __alloc_skb+0x7e/0x120 [] netpoll_send_udp+0x1dc/0x1f0 [] write_msg+0x8f/0xc0 [netconsole] [] ? store_remote_port+0x60/0x60 [netconsole] [] __call_console_drivers+0x77/0x90 [] _call_console_drivers+0x51/0x90 [] call_console_drivers+0x8b/0xd0 [] console_unlock+0x37/0xc0 [] vprintk+0x159/0x300 [] ? path_openat+0xc5/0x360 [] ? do_filp_open+0x35/0x80 [] printk+0x20/0x30 [] __handle_sysrq+0x3d/0x110 [] ? security_file_permission+0x1e/0x90 [] write_sysrq_trigger+0x4a/0x50 [] ? __handle_sysrq+0x110/0x110 [] proc_reg_write+0x5d/0x80 [] vfs_write+0x9b/0x160 [] ? audit_syscall_exit+0x381/0x3f0 [] ? proc_reg_poll+0x80/0x80 [] sys_write+0x42/0x70 [] sysenter_do_call+0x12/0x28 Maxim.