* Re: qlge warning
From: Jarek Poplawski @ 2010-12-03 20:56 UTC (permalink / raw)
To: Yinghai Lu; +Cc: David Miller, NetDev, Ingo Molnar, Ron Mercer, linux-driver
In-Reply-To: <AANLkTi=oGgajs5KX9haKUALprJTatctpGs5UNU7LrRGO@mail.gmail.com>
It looks like cancel_delayed_work_sync in ql_adapter_down is illegal.
We can't sync works with rtnl_lock while holding it in qlge_close.
Maintainers CC'ed.
Jarek P.
Yinghai Lu wrote:
> [ 290.233264] =======================================================
> [ 290.251780] [ INFO: possible circular locking dependency detected ]
> [ 290.271534] 2.6.37-rc4-tip-yh-05919-geb30094-dirty #308
> [ 290.271775] -------------------------------------------------------
> [ 290.291512] swapper/1 is trying to acquire lock:
> [ 290.291725] ((&(&qdev->mpi_port_cfg_work)->work)){+.+...}, at:
> [<ffffffff81096419>] wait_on_work+0x0/0xff
> [ 290.311643]
> [ 290.311644] but task is already holding lock:
> [ 290.311915] (rtnl_mutex){+.+.+.}, at: [<ffffffff81bb094d>]
> rtnl_lock+0x17/0x19
> [ 290.331681]
> [ 290.331682] which lock already depends on the new lock.
> [ 290.331684]
> [ 290.351491]
> [ 290.351492] the existing dependency chain (in reverse order) is:
> [ 290.351830]
> [ 290.351831] -> #1 (rtnl_mutex){+.+.+.}:
> [ 290.371562] [<ffffffff810ae6b6>] lock_acquire+0xca/0xf0
> [ 290.371824] [<ffffffff81cdbf5d>] mutex_lock_nested+0x60/0x2b8
> [ 290.391539] [<ffffffff81bb094d>] rtnl_lock+0x17/0x19
> [ 290.411250] [<ffffffff818501ad>] ql_mpi_port_cfg_work+0x1f/0x1ad
> [ 290.411606] [<ffffffff81095189>] process_one_work+0x234/0x3e8
> [ 290.431282] [<ffffffff81095663>] worker_thread+0x17f/0x261
> [ 290.431583] [<ffffffff8109a633>] kthread+0xa0/0xa8
> [ 290.451279] [<ffffffff8103a914>] kernel_thread_helper+0x4/0x10
> [ 290.451581]
> [ 290.451582] -> #0 ((&(&qdev->mpi_port_cfg_work)->work)){+.+...}:
> [ 290.471483] [<ffffffff810ada85>] __lock_acquire+0x113c/0x1813
> [ 290.491177] [<ffffffff810ae6b6>] lock_acquire+0xca/0xf0
> [ 290.491451] [<ffffffff8109646c>] wait_on_work+0x53/0xff
> [ 290.511128] [<ffffffff810965da>] __cancel_work_timer+0xc2/0x102
> [ 290.511434] [<ffffffff8109662c>] cancel_delayed_work_sync+0x12/0x14
> [ 290.531233] [<ffffffff81847646>] ql_cancel_all_work_sync+0x64/0x68
> [ 290.531563] [<ffffffff818499d5>] ql_adapter_down+0x23/0xf6
> [ 290.551298] [<ffffffff81849ca7>] qlge_close+0x67/0x76
> [ 290.571015] [<ffffffff81ba3853>] __dev_close+0x7b/0x89
> [ 290.571297] [<ffffffff81ba5535>] __dev_change_flags+0xad/0x131
> [ 290.590974] [<ffffffff81ba563a>] dev_change_flags+0x21/0x57
> [ 290.591280] [<ffffffff827de30e>] ic_close_devs+0x2e/0x48
> [ 290.610978] [<ffffffff827df332>] ip_auto_config+0xbc9/0xe84
> [ 290.611280] [<ffffffff810002da>] do_one_initcall+0x57/0x135
> [ 290.630977] [<ffffffff8278ef8a>] kernel_init+0x16c/0x1f6
> [ 290.631263] [<ffffffff8103a914>] kernel_thread_helper+0x4/0x10
> [ 290.651000]
> [ 290.651001] other info that might help us debug this:
> [ 290.651003]
> [ 290.670829] 1 lock held by swapper/1:
> [ 290.671013] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff81bb094d>]
> rtnl_lock+0x17/0x19
> [ 290.690819]
> [ 290.690820] stack backtrace:
> [ 290.691054] Pid: 1, comm: swapper Not tainted
> 2.6.37-rc4-tip-yh-05919-geb30094-dirty #308
> [ 290.710805] Call Trace:
> [ 290.710938] [<ffffffff810aa296>] ? print_circular_bug+0xaf/0xbe
> [ 290.730683] [<ffffffff810ada85>] ? __lock_acquire+0x113c/0x1813
> [ 290.730955] [<ffffffff81095d70>] ? wait_on_cpu_work+0xdb/0x114
> [ 290.750672] [<ffffffff81096419>] ? wait_on_work+0x0/0xff
> [ 290.750939] [<ffffffff810ae6b6>] ? lock_acquire+0xca/0xf0
> [ 290.770664] [<ffffffff81096419>] ? wait_on_work+0x0/0xff
> [ 290.770920] [<ffffffff8109646c>] ? wait_on_work+0x53/0xff
> [ 290.790575] [<ffffffff81096419>] ? wait_on_work+0x0/0xff
> [ 290.790821] [<ffffffff810965da>] ? __cancel_work_timer+0xc2/0x102
> [ 290.810559] [<ffffffff8109662c>] ? cancel_delayed_work_sync+0x12/0x14
> [ 290.810855] [<ffffffff81847646>] ? ql_cancel_all_work_sync+0x64/0x68
> [ 290.830594] [<ffffffff818499d5>] ? ql_adapter_down+0x23/0xf6
> [ 290.830867] [<ffffffff81849ca7>] ? qlge_close+0x67/0x76
> [ 290.850568] [<ffffffff81ba3853>] ? __dev_close+0x7b/0x89
> [ 290.850829] [<ffffffff81ba5535>] ? __dev_change_flags+0xad/0x131
> [ 290.870540] [<ffffffff81ba563a>] ? dev_change_flags+0x21/0x57
> [ 290.870815] [<ffffffff827de30e>] ? ic_close_devs+0x2e/0x48
> [ 290.890595] [<ffffffff827df332>] ? ip_auto_config+0xbc9/0xe84
> [ 290.910247] [<ffffffff81cda1e3>] ? printk+0x41/0x43
> [ 290.910488] [<ffffffff827de769>] ? ip_auto_config+0x0/0xe84
> [ 290.910747] [<ffffffff810002da>] ? do_one_initcall+0x57/0x135
> [ 290.930455] [<ffffffff8278ef8a>] ? kernel_init+0x16c/0x1f6
> [ 290.930743] [<ffffffff8103a914>] ? kernel_thread_helper+0x4/0x10
> [ 290.950419] [<ffffffff81cde23c>] ? restore_args+0x0/0x30
> [ 290.970152] [<ffffffff8278ee1e>] ? kernel_init+0x0/0x1f6
> [ 290.970398] [<ffffffff8103a910>] ? kernel_thread_helper+0x0/0x10
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Jarek Poplawski @ 2010-12-03 22:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrej Ota, linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <1291387595.2897.350.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le vendredi 03 décembre 2010 à 15:37 +0100, Andrej Ota a écrit :
>>>> Patch that works for me is below. Now I only hope I haven't
>>>> (re)introduced a memory leak...
>>
>>> Problem comes from commit 55c95e738da85 (fix return value of
>>> __pppoe_xmit() method)
...
> Rami, could you re-explain the rationale of your patch ?
I guess we could revert it in the meantime: it seems there might be
at least three threads (including bugzilla) with this problem.
Jarek P.
^ permalink raw reply
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Denys Fedoryshchenko @ 2010-12-03 22:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrej Ota, linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <1291387595.2897.350.camel@edumazet-laptop>
On Friday 03 December 2010 16:46:35 Eric Dumazet wrote:
> Le vendredi 03 décembre 2010 à 15:37 +0100, Andrej Ota a écrit :
> > >> Patch that works for me is below. Now I only hope I haven't
> > >> (re)introduced a memory leak...
> > >
> > > Problem comes from commit 55c95e738da85 (fix return value of
> > > __pppoe_xmit() method)
> > >
> > > I am not sure patch is OK
> >
> > Me neither. That's why I wrote "works for me". All I dare say is that it
> > works better than current code and is probably no worse than it was
> > before above mentioned commit. Apart from that, there is no point in
> > having return value for __pppoe_xmit if return value isn't needed.
> >
> > Easiest way of triggering this BUG is by terminating PPPoE on the server
> > side, which then hits "if (!dev) { goto abort; }". This in turn calls
> > "kfree_skb(skb); return 0;" which returns to pppoe_rcv_core which then
> > goto-s to "abort_put" which again calls "kfree_skb(skb)". Voila the bug.
> >
> > I don't know how to trigger "if (skb_cow_head(skb, ..." to see if I have
> > just caused another BUG. However, if I read file comments at the top, I
> > see a comment from 19/07/01 stating that I have to delete original skb
> > if code succeeds and never delete it on failure. About the skb copy
> > mentioned in the same comment, I don't know. 2001 was many commits ago.
>
> Well, all I wanted to say was that _I_ was not sure, but probably other
> network guys have a better diagnostic.
>
> Rami, could you re-explain the rationale of your patch ?
>
> Thanks
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
This patch seems fixed my issue (Re: 2.6.35->2.6.36 regression, vanilla kernel
panic, ppp or hrtimers crashing), tested now.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ixgbe: fix link behavior for SFP+ when driver is brought down
From: Kirsher, Jeffrey T @ 2010-12-03 22:44 UTC (permalink / raw)
To: davem@davemloft.net
Cc: Skidmore, Donald C, netdev@vger.kernel.org, gospo@redhat.com,
bphilips@novell.com
In-Reply-To: <20101203.094215.229763630.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 810 bytes --]
On Fri, 2010-12-03 at 09:42 -0800, David Miller wrote:
> Please number your patches so I know unambiguously in which order to
> apply them.
>
> The last patch didn't apply cleanly because ixgbe_types.h in your
> tree has:
>
> #define IXGBE_ERR_SFP_SETUP_NOT_COMPLETE -30
>
> but that does not exist in net-next-2.6 This doesn't exist in
> net-2.6 either so it's something completely local to your trees.
>
> I fixed all of this up and applied it all, but this sort of stuff
> needs to be sorted out before I see the submission. :-)
I apologize, the missing define was in the patch
http://patchwork.ozlabs.org/patch/73942/
which I sent out earlier this week. I was going to send you out a
summary email letting you know that the six patches I submitted were on
my tree. My fault that I did not.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 491 bytes --]
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ixgbe: fix link behavior for SFP+ when driver is brought down
From: Kirsher, Jeffrey T @ 2010-12-03 22:46 UTC (permalink / raw)
To: davem@davemloft.net
Cc: Skidmore, Donald C, netdev@vger.kernel.org, gospo@redhat.com,
bphilips@novell.com
In-Reply-To: <20101203.101512.15225225.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]
On Fri, 2010-12-03 at 10:15 -0800, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Fri, 03 Dec 2010 09:42:15 -0800 (PST)
>
> > I fixed all of this up and applied it all, but this sort of stuff
> > needs to be sorted out before I see the submission. :-)
>
> These patches introduced the following new compile warnings,
> please send me patches to fix them:
>
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_up_complete':
> drivers/net/ixgbe/ixgbe_main.c:3803:22: warning: comparison between 'enum ixgbe_phy_type' and 'enum ixgbe_media_type'
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_down':
> drivers/net/ixgbe/ixgbe_main.c:4125:22: warning: comparison between 'enum ixgbe_phy_type' and 'enum ixgbe_media_type'
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_probe':
> drivers/net/ixgbe/ixgbe_main.c:7215:22: warning: comparison between 'enum ixgbe_phy_type' and 'enum ixgbe_media_type'
>
> Thanks.
Very interesting that we did not see this during our compile testing of
the patch. Don is working on a patch right now to fix the issue.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 491 bytes --]
^ permalink raw reply
* Re: jme: UDP checksum error, and lots of them
From: Francois Romieu @ 2010-12-03 23:02 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: David Miller, cooldavid, netdev
In-Reply-To: <alpine.LNX.2.01.1012030044440.22334@obet.zrqbmnf.qr>
Jan Engelhardt <jengelh@medozas.de> :
> On Friday 2010-12-03 00:15, Francois Romieu wrote:
> >David Miller <davem@davemloft.net> :
> >[...]
> >> Something isn't right here. The only thing that makes sense is if
> >> the tcpdump checksum validation is wrong for some reason. Because
> >> only then could we give a reason for the UDP frames to not be
> >> dropped before vpnc can see them.
> >
> >Wild guess : 192.168... is the local address and tcpdump chokes on
> >an outgoing, yet-not-checksummed packet.
>
> Then the checksums of all other packets I am emitting would also be 0000
> or wrong, which isn't the case.
No. :o)
net/ipv4/udp.c::udp4_hwcsum_outgoing provides a packet data length
dependent partial checksum and it checksums fragments.
--
Ueimor
^ permalink raw reply
* [net-next-2.6 PATCH 1/2] ixgbe: fix for link failure on SFP+ DA cables
From: Jeff Kirsher @ 2010-12-03 23:23 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
From: Don Skidmore <donald.c.skidmore@intel.com>
This patch helps prevent FW/SW semaphore collision from leading
to link establishment failure. The collision might mess up the
PHY registers so we reset the PHY. However there are SFI/KR areas
in the PHY that are not reset with a Reset_AN so we need to change
LMS to reset it. Also wait until AN state machine is AN_GOOD
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_82599.c | 28 +++++++++++++++++++++++++---
drivers/net/ixgbe/ixgbe_type.h | 3 +++
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 385cceb..6827ddd 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -96,6 +96,8 @@ static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
{
s32 ret_val = 0;
+ u32 reg_anlp1 = 0;
+ u32 i = 0;
u16 list_offset, data_offset, data_value;
if (hw->phy.sfp_type != ixgbe_sfp_type_unknown) {
@@ -122,14 +124,34 @@ static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
IXGBE_WRITE_FLUSH(hw);
hw->eeprom.ops.read(hw, ++data_offset, &data_value);
}
- /* Now restart DSP by setting Restart_AN */
- IXGBE_WRITE_REG(hw, IXGBE_AUTOC,
- (IXGBE_READ_REG(hw, IXGBE_AUTOC) | IXGBE_AUTOC_AN_RESTART));
/* Release the semaphore */
ixgbe_release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
/* Delay obtaining semaphore again to allow FW access */
msleep(hw->eeprom.semaphore_delay);
+
+ /* Now restart DSP by setting Restart_AN and clearing LMS */
+ IXGBE_WRITE_REG(hw, IXGBE_AUTOC, ((IXGBE_READ_REG(hw,
+ IXGBE_AUTOC) & ~IXGBE_AUTOC_LMS_MASK) |
+ IXGBE_AUTOC_AN_RESTART));
+
+ /* Wait for AN to leave state 0 */
+ for (i = 0; i < 10; i++) {
+ msleep(4);
+ reg_anlp1 = IXGBE_READ_REG(hw, IXGBE_ANLP1);
+ if (reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)
+ break;
+ }
+ if (!(reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)) {
+ hw_dbg(hw, "sfp module setup not complete\n");
+ ret_val = IXGBE_ERR_SFP_SETUP_NOT_COMPLETE;
+ goto setup_sfp_out;
+ }
+
+ /* Restart DSP by setting Restart_AN and return to SFI mode */
+ IXGBE_WRITE_REG(hw, IXGBE_AUTOC, (IXGBE_READ_REG(hw,
+ IXGBE_AUTOC) | IXGBE_AUTOC_LMS_10G_SERIAL |
+ IXGBE_AUTOC_AN_RESTART));
}
setup_sfp_out:
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index ef816dd..0f80893 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -1470,6 +1470,8 @@
#define IXGBE_ANLP1_PAUSE 0x0C00
#define IXGBE_ANLP1_SYM_PAUSE 0x0400
#define IXGBE_ANLP1_ASM_PAUSE 0x0800
+#define IXGBE_ANLP1_AN_STATE_MASK 0x000f0000
+
/* SW Semaphore Register bitmasks */
#define IXGBE_SWSM_SMBI 0x00000001 /* Driver Semaphore bit */
@@ -2641,6 +2643,7 @@ struct ixgbe_info {
#define IXGBE_ERR_NO_SPACE -25
#define IXGBE_ERR_OVERTEMP -26
#define IXGBE_ERR_RAR_INDEX -27
+#define IXGBE_ERR_SFP_SETUP_NOT_COMPLETE -30
#define IXGBE_ERR_PBA_SECTION -31
#define IXGBE_ERR_INVALID_ARGUMENT -32
#define IXGBE_NOT_IMPLEMENTED 0x7FFFFFFF
^ permalink raw reply related
* [net-next-2.6 PATCH 2/2] ixgbe: fix enum type mismatch on disable laser
From: Jeff Kirsher @ 2010-12-03 23:24 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Jeff Kirsher
In-Reply-To: <20101203232218.21869.62349.stgit@localhost.localdomain>
From: Don Skidmore <donald.c.skidmore@intel.com>
Fixes a recent bug on the patch (c6ecf39a10ceec3e97096e2a8d3eadcecd593422)
that disabled the laser on ifconfig down. Compilers were seeing a enum
mismatch.
Signed-off-by Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index c5998ca..fdb35d0 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -3800,7 +3800,7 @@ static int ixgbe_up_complete(struct ixgbe_adapter *adapter)
/* enable the optics for both mult-speed fiber and 82599 SFP+ fiber */
if (hw->mac.ops.enable_tx_laser &&
((hw->phy.multispeed_fiber) ||
- ((hw->phy.type == ixgbe_media_type_fiber) &&
+ ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
(hw->mac.type == ixgbe_mac_82599EB))))
hw->mac.ops.enable_tx_laser(hw);
@@ -4122,7 +4122,7 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
/* power down the optics for multispeed fiber and 82599 SFP+ fiber */
if (hw->mac.ops.disable_tx_laser &&
((hw->phy.multispeed_fiber) ||
- ((hw->phy.type == ixgbe_media_type_fiber) &&
+ ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
(hw->mac.type == ixgbe_mac_82599EB))))
hw->mac.ops.disable_tx_laser(hw);
@@ -7215,7 +7215,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
/* power down the optics for multispeed fiber and 82599 SFP+ fiber */
if (hw->mac.ops.disable_tx_laser &&
((hw->phy.multispeed_fiber) ||
- ((hw->phy.type == ixgbe_media_type_fiber) &&
+ ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
(hw->mac.type == ixgbe_mac_82599EB))))
hw->mac.ops.disable_tx_laser(hw);
^ permalink raw reply related
* [PATCH 1/1] TCP: Bug fix in initialization of receive window.
From: Nandita Dukkipati @ 2010-12-03 23:33 UTC (permalink / raw)
To: David S. Miller; +Cc: Laurent Chavey, Yuchung Cheng, netdev, Nandita Dukkipati
The bug has to do with boundary checks on the initial receive window.
If the initial receive window falls between init_cwnd and the
receive window specified by the user, the initial window is incorrectly
brought down to init_cwnd. The correct behavior is to allow it to
remain unchanged.
Signed-off-by: Nandita Dukkipati <nanditad@google.com>
---
net/ipv4/tcp_output.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 749b649..31fd0bd 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -231,11 +231,10 @@ void tcp_select_initial_window(int __space, __u32 mss,
/* when initializing use the value from init_rcv_wnd
* rather than the default from above
*/
- if (init_rcv_wnd &&
- (*rcv_wnd > init_rcv_wnd * mss))
- *rcv_wnd = init_rcv_wnd * mss;
- else if (*rcv_wnd > init_cwnd * mss)
- *rcv_wnd = init_cwnd * mss;
+ if (init_rcv_wnd)
+ *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
+ else
+ *rcv_wnd = min(*rcv_wnd, init_cwnd * mss);
}
/* Set the clamp no higher than max representable value */
--
1.7.3.1
^ permalink raw reply related
* [PATCH 0/2] Add and use vsprintf extension %pMbt for bluetooth macs
From: Joe Perches @ 2010-12-04 2:33 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo F. Padovan,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Using vsprintf extensions can save text and data.
Add %pMbt for the byte reversed output for bluetooth addresses.
Joe Perches (2):
vsprintf: Add %pMbt, bluetooth mac address
bluetooth: Use printf extension %pMbt
lib/vsprintf.c | 6 +++++-
net/bluetooth/bnep/core.c | 3 +--
net/bluetooth/cmtp/core.c | 2 +-
net/bluetooth/hci_conn.c | 6 +++---
net/bluetooth/hci_core.c | 8 ++++----
net/bluetooth/hci_event.c | 6 +++---
net/bluetooth/hci_sysfs.c | 10 +++++-----
net/bluetooth/hidp/core.c | 4 ++--
net/bluetooth/l2cap.c | 19 +++++++++----------
net/bluetooth/lib.c | 14 --------------
net/bluetooth/rfcomm/core.c | 16 ++++++++--------
net/bluetooth/rfcomm/sock.c | 8 ++++----
net/bluetooth/rfcomm/tty.c | 6 +++---
net/bluetooth/sco.c | 12 ++++++------
14 files changed, 54 insertions(+), 66 deletions(-)
--
1.7.3.2.245.g03276.dirty
^ permalink raw reply
* [PATCH 1/2] vsprintf: Add %pMbt, bluetooth mac address
From: Joe Perches @ 2010-12-04 2:33 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo F. Padovan, linux-kernel; +Cc: netdev
In-Reply-To: <cover.1291419007.git.joe@perches.com>
Bluetooth output the MAC address in reverse order.
Bluetooth memory order: 00 01 02 03 04 05 is output "05:04:03:02:01:00".
This can save overall text when bluetooth is compiled in.
Bluetooth currently uses a very slightly unsafe local function (batostr)
to output these formatted addresses.
Adding %pMbt allows the batostr function to be removed.
For x86:
$ size lib/vsprintf*.o*
text data bss dec hex filename
8189 0 2 8191 1fff lib/vsprintf.o.defconfig.new
8150 0 2 8152 1fd8 lib/vsprintf.o.defconfig.old
18633 56 3936 22625 5861 lib/vsprintf.o.allyesconfig.new
18571 56 3920 22547 5813 lib/vsprintf.o.allyesconfig.old
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index c150d3d..9346ed9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -700,15 +700,18 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
char *p = mac_addr;
int i;
char separator;
+ bool bluetooth = false;
if (fmt[1] == 'F') { /* FDDI canonical format */
separator = '-';
} else {
separator = ':';
+ if (fmt[1] == 'b' && fmt[2] == 't')
+ bluetooth = true;
}
for (i = 0; i < 6; i++) {
- p = pack_hex_byte(p, addr[i]);
+ p = pack_hex_byte(p, addr[!bluetooth ? i : 5 - i]);
if (fmt[0] == 'M' && i != 5)
*p++ = separator;
}
@@ -1012,6 +1015,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'M': /* Colon separated: 00:01:02:03:04:05 */
case 'm': /* Contiguous: 000102030405 */
/* [mM]F (FDDI, bit reversed) */
+ /* [mM]bt (Bluetooth, index:543210) */
return mac_address_string(buf, end, ptr, spec, fmt);
case 'I': /* Formatted IP supported
* 4: 1.2.3.4
--
1.7.3.2.245.g03276.dirty
^ permalink raw reply related
* [PATCH 2/2] bluetooth: Use printf extension %pMbt
From: Joe Perches @ 2010-12-04 2:33 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo F. Padovan
Cc: netdev, David S. Miller, linux-bluetooth, linux-kernel
In-Reply-To: <cover.1291419007.git.joe@perches.com>
Save some text and bss.
Remove function batostr so there's no possibility of bad output.
from the net/bluetooth directory:
$ size built-in.o.*
text data bss dec hex filename
293562 16265 70088 379915 5cc0b built-in.o.allyesconfig.new
294619 16269 70480 381368 5d1b8 built-in.o.allyesconfig.old
30359 772 56 31187 79d3 built-in.o.btonly.new
30555 776 92 31423 7abf built-in.o.btonly.old
Signed-off-by: Joe Perches <joe@perches.com>
---
net/bluetooth/bnep/core.c | 3 +--
net/bluetooth/cmtp/core.c | 2 +-
net/bluetooth/hci_conn.c | 6 +++---
net/bluetooth/hci_core.c | 8 ++++----
net/bluetooth/hci_event.c | 6 +++---
net/bluetooth/hci_sysfs.c | 10 +++++-----
net/bluetooth/hidp/core.c | 4 ++--
net/bluetooth/l2cap.c | 19 +++++++++----------
net/bluetooth/lib.c | 14 --------------
net/bluetooth/rfcomm/core.c | 16 ++++++++--------
net/bluetooth/rfcomm/sock.c | 8 ++++----
net/bluetooth/rfcomm/tty.c | 6 +++---
net/bluetooth/sco.c | 12 ++++++------
13 files changed, 49 insertions(+), 65 deletions(-)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 5868597..3d4530f 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -199,8 +199,7 @@ static int bnep_ctrl_set_mcfilter(struct bnep_session *s, u8 *data, int len)
memcpy(a1, data, ETH_ALEN); data += ETH_ALEN;
a2 = data; data += ETH_ALEN;
- BT_DBG("mc filter %s -> %s",
- batostr((void *) a1), batostr((void *) a2));
+ BT_DBG("mc filter %pMbt -> %pMbt", a1, a2);
#define INCA(a) { int i = 5; while (i >=0 && ++a[i--] == 0); }
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 8e5f292..f72bca7 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -344,7 +344,7 @@ int cmtp_add_connection(struct cmtp_connadd_req *req, struct socket *sock)
BT_DBG("mtu %d", session->mtu);
- sprintf(session->name, "%s", batostr(&bt_sk(sock->sk)->dst));
+ sprintf(session->name, "%pMbt", &bt_sk(sock->sk)->dst);
session->sock = sock;
session->state = BT_CONFIG;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0b1e460..d9e3eb3 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -221,7 +221,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
{
struct hci_conn *conn;
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
+ BT_DBG("%s dst %pMbt", hdev->name, dst);
conn = kzalloc(sizeof(struct hci_conn), GFP_ATOMIC);
if (!conn)
@@ -325,7 +325,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
struct hci_dev *hdev = NULL;
struct list_head *p;
- BT_DBG("%s -> %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt -> %pMbt", src, dst);
read_lock_bh(&hci_dev_list_lock);
@@ -366,7 +366,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
struct hci_conn *acl;
struct hci_conn *sco;
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
+ BT_DBG("%s dst %pMbt", hdev->name, dst);
if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc2a052..47962af 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -338,7 +338,7 @@ struct inquiry_entry *hci_inquiry_cache_lookup(struct hci_dev *hdev, bdaddr_t *b
struct inquiry_cache *cache = &hdev->inq_cache;
struct inquiry_entry *e;
- BT_DBG("cache %p, %s", cache, batostr(bdaddr));
+ BT_DBG("cache %p, %pMbt", cache, bdaddr);
for (e = cache->list; e; e = e->next)
if (!bacmp(&e->data.bdaddr, bdaddr))
@@ -351,7 +351,7 @@ void hci_inquiry_cache_update(struct hci_dev *hdev, struct inquiry_data *data)
struct inquiry_cache *cache = &hdev->inq_cache;
struct inquiry_entry *e;
- BT_DBG("cache %p, %s", cache, batostr(&data->bdaddr));
+ BT_DBG("cache %p, %pMbt", cache, &data->bdaddr);
if (!(e = hci_inquiry_cache_lookup(hdev, &data->bdaddr))) {
/* Entry not in the cache. Add new one. */
@@ -1478,8 +1478,8 @@ static inline void hci_acl_tx_to(struct hci_dev *hdev)
list_for_each(p, &h->list) {
c = list_entry(p, struct hci_conn, list);
if (c->type == ACL_LINK && c->sent) {
- BT_ERR("%s killing stalled ACL connection %s",
- hdev->name, batostr(&c->dst));
+ BT_ERR("%s killing stalled ACL connection %pMbt",
+ hdev->name, &c->dst);
hci_acl_disconn(c, 0x13);
}
}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3c1957c..53f833f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -566,7 +566,7 @@ static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
- BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->bdaddr), conn);
+ BT_DBG("%s bdaddr %pMbt conn %p", hdev->name, &cp->bdaddr, conn);
if (status) {
if (conn && conn->state == BT_CONNECT) {
@@ -984,8 +984,8 @@ static inline void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *sk
struct hci_ev_conn_request *ev = (void *) skb->data;
int mask = hdev->link_mode;
- BT_DBG("%s bdaddr %s type 0x%x", hdev->name,
- batostr(&ev->bdaddr), ev->link_type);
+ BT_DBG("%s bdaddr %pMbt type 0x%x",
+ hdev->name, &ev->bdaddr, ev->link_type);
mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 5fce3d6..5dac407 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -37,7 +37,7 @@ static ssize_t show_link_type(struct device *dev, struct device_attribute *attr,
static ssize_t show_link_address(struct device *dev, struct device_attribute *attr, char *buf)
{
struct hci_conn *conn = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", batostr(&conn->dst));
+ return sprintf(buf, "%pMbt\n", &conn->dst);
}
static ssize_t show_link_features(struct device *dev, struct device_attribute *attr, char *buf)
@@ -236,7 +236,7 @@ static ssize_t show_class(struct device *dev, struct device_attribute *attr, cha
static ssize_t show_address(struct device *dev, struct device_attribute *attr, char *buf)
{
struct hci_dev *hdev = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", batostr(&hdev->bdaddr));
+ return sprintf(buf, "%pMbt\n", &hdev->bdaddr);
}
static ssize_t show_features(struct device *dev, struct device_attribute *attr, char *buf)
@@ -404,8 +404,8 @@ static int inquiry_cache_show(struct seq_file *f, void *p)
for (e = cache->list; e; e = e->next) {
struct inquiry_data *data = &e->data;
- seq_printf(f, "%s %d %d %d 0x%.2x%.2x%.2x 0x%.4x %d %d %u\n",
- batostr(&data->bdaddr),
+ seq_printf(f, "%pMbt %d %d %d 0x%.2x%.2x%.2x 0x%.4x %d %d %u\n",
+ &data->bdaddr,
data->pscan_rep_mode, data->pscan_period_mode,
data->pscan_mode, data->dev_class[2],
data->dev_class[1], data->dev_class[0],
@@ -442,7 +442,7 @@ static int blacklist_show(struct seq_file *f, void *p)
b = list_entry(l, struct bdaddr_list, list);
- seq_printf(f, "%s\n", batostr(&b->bdaddr));
+ seq_printf(f, "%pMbt\n", &b->bdaddr);
}
hci_dev_unlock_bh(hdev);
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 29544c2..cde8827 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -787,8 +787,8 @@ static int hidp_setup_hid(struct hidp_session *session,
hid->country = req->country;
strncpy(hid->name, req->name, 128);
- strncpy(hid->phys, batostr(&bt_sk(session->ctrl_sock->sk)->src), 64);
- strncpy(hid->uniq, batostr(&bt_sk(session->ctrl_sock->sk)->dst), 64);
+ snprintf(hid->phys, 64, "%pMbt", &bt_sk(session->ctrl_sock->sk)->src);
+ snprintf(hid->uniq, 64, "%pMbt", &bt_sk(session->ctrl_sock->sk)->dst);
hid->dev.parent = hidp_get_device(session);
hid->ll_driver = &hidp_hid_driver;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 12b4aa2..72b8306 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1057,8 +1057,7 @@ static int l2cap_do_connect(struct sock *sk)
__u8 auth_type;
int err;
- BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
- l2cap_pi(sk)->psm);
+ BT_DBG("%pMbt -> %pMbt psm 0x%2.2x", src, dst, l2cap_pi(sk)->psm);
hdev = hci_get_route(dst, src);
if (!hdev)
@@ -4525,7 +4524,7 @@ static int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
if (type != ACL_LINK)
return -EINVAL;
- BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
+ BT_DBG("hdev %s, bdaddr %pMbt", hdev->name, bdaddr);
/* Find listening sockets and check their link_mode */
read_lock(&l2cap_sk_list.lock);
@@ -4553,7 +4552,7 @@ static int l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
{
struct l2cap_conn *conn;
- BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
+ BT_DBG("hcon %p bdaddr %pMbt status %d", hcon, &hcon->dst, status);
if (hcon->type != ACL_LINK)
return -EINVAL;
@@ -4798,12 +4797,12 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
sk_for_each(sk, node, &l2cap_sk_list.head) {
struct l2cap_pinfo *pi = l2cap_pi(sk);
- seq_printf(f, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
- sk->sk_state, __le16_to_cpu(pi->psm),
- pi->scid, pi->dcid,
- pi->imtu, pi->omtu, pi->sec_level);
+ seq_printf(f, "%pMbt %pMbt %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
+ sk->sk_state, __le16_to_cpu(pi->psm),
+ pi->scid, pi->dcid,
+ pi->imtu, pi->omtu, pi->sec_level);
}
read_unlock_bh(&l2cap_sk_list.lock);
diff --git a/net/bluetooth/lib.c b/net/bluetooth/lib.c
index b826d1b..bfc8bbb 100644
--- a/net/bluetooth/lib.c
+++ b/net/bluetooth/lib.c
@@ -44,20 +44,6 @@ void baswap(bdaddr_t *dst, bdaddr_t *src)
}
EXPORT_SYMBOL(baswap);
-char *batostr(bdaddr_t *ba)
-{
- static char str[2][18];
- static int i = 1;
-
- i ^= 1;
- sprintf(str[i], "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- ba->b[5], ba->b[4], ba->b[3],
- ba->b[2], ba->b[1], ba->b[0]);
-
- return str[i];
-}
-EXPORT_SYMBOL(batostr);
-
/* Bluetooth error codes to Unix errno mapping */
int bt_err(__u16 code)
{
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fa642aa..532e5ad 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -393,8 +393,8 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
int err = 0;
u8 dlci;
- BT_DBG("dlc %p state %ld %s %s channel %d",
- d, d->state, batostr(src), batostr(dst), channel);
+ BT_DBG("dlc %p state %ld %pMbt %pMbt channel %d",
+ d, d->state, src, dst, channel);
if (channel < 1 || channel > 30)
return -EINVAL;
@@ -692,7 +692,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
struct socket *sock;
struct sock *sk;
- BT_DBG("%s %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt %pMbt", src, dst);
*err = rfcomm_l2sock_create(&sock);
if (*err < 0)
@@ -2120,11 +2120,11 @@ static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
struct sock *sk = s->sock->sk;
struct rfcomm_dlc *d = list_entry(pp, struct rfcomm_dlc, list);
- seq_printf(f, "%s %s %ld %d %d %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
- d->state, d->dlci, d->mtu,
- d->rx_credits, d->tx_credits);
+ seq_printf(f, "%pMbt %pMbt %ld %d %d %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
+ d->state, d->dlci, d->mtu,
+ d->rx_credits, d->tx_credits);
}
}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 0207bd6..bbbc7479 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -350,7 +350,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
struct sock *sk = sock->sk;
int err = 0;
- BT_DBG("sk %p %s", sk, batostr(&sa->rc_bdaddr));
+ BT_DBG("sk %p %pMbt", sk, &sa->rc_bdaddr);
if (!addr || addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
@@ -979,9 +979,9 @@ static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
read_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
- seq_printf(f, "%s %s %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
+ seq_printf(f, "%pMbt %pMbt %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
sk->sk_state, rfcomm_pi(sk)->channel);
}
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a9b81f5..740c99b 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -183,7 +183,7 @@ static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
{
struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
- return sprintf(buf, "%s\n", batostr(&dev->dst));
+ return sprintf(buf, "%pMbt\n", &dev->dst);
}
static ssize_t show_channel(struct device *tty_dev, struct device_attribute *attr, char *buf)
@@ -685,8 +685,8 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
if (!dev)
return -ENODEV;
- BT_DBG("dev %p dst %s channel %d opened %d", dev, batostr(&dev->dst),
- dev->channel, atomic_read(&dev->opened));
+ BT_DBG("dev %p dst %pMbt channel %d opened %d",
+ dev, &dev->dst, dev->channel, atomic_read(&dev->opened));
if (atomic_inc_return(&dev->opened) > 1)
return 0;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 66b9e5c..ac8370e 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -183,7 +183,7 @@ static int sco_connect(struct sock *sk)
struct hci_dev *hdev;
int err, type;
- BT_DBG("%s -> %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt -> %pMbt", src, dst);
if (!(hdev = hci_get_route(dst, src)))
return -EHOSTUNREACH;
@@ -457,7 +457,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
bdaddr_t *src = &sa->sco_bdaddr;
int err = 0;
- BT_DBG("sk %p %s", sk, batostr(&sa->sco_bdaddr));
+ BT_DBG("sk %p %pMbt", sk, src);
if (!addr || addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
@@ -884,7 +884,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
if (type != SCO_LINK && type != ESCO_LINK)
return -EINVAL;
- BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
+ BT_DBG("hdev %s, bdaddr %pMbt", hdev->name, bdaddr);
/* Find listening sockets */
read_lock(&sco_sk_list.lock);
@@ -905,7 +905,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
static int sco_connect_cfm(struct hci_conn *hcon, __u8 status)
{
- BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
+ BT_DBG("hcon %p bdaddr %pMbt status %d", hcon, &hcon->dst, status);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
return -EINVAL;
@@ -961,8 +961,8 @@ static int sco_debugfs_show(struct seq_file *f, void *p)
read_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head) {
- seq_printf(f, "%s %s %d\n", batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst), sk->sk_state);
+ seq_printf(f, "%pMbt %pMbt %d\n",
+ &bt_sk(sk)->src, &bt_sk(sk)->dst, sk->sk_state);
}
read_unlock_bh(&sco_sk_list.lock);
--
1.7.3.2.245.g03276.dirty
^ permalink raw reply related
* [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 5:45 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, Tom Herbert, Jiri Pirko, netdev, Changli Gao
The dev field of ingress queue is forgot to initialized, then NULL
pointer dereference happens in qdisc_alloc().
[ 303.019348] BUG: unable to handle kernel NULL pointer dereference at 0000000000000398
[ 303.020068] IP: [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] PGD 3d637067 PUD 3d03f067 PMD 0
[ 303.020068] Oops: 0000 [#1] SMP
[ 303.020068] last sysfs file: /sys/kernel/uevent_seqnum
[ 303.020068] CPU 0
[ 303.020068] Modules linked in: sch_ingress ipv6
[ 303.020068]
[ 303.020068] Pid: 3138, comm: tc Not tainted 2.6.37-rc1+ #90 /VirtualBox
[ 303.020068] RIP: 0010:[<ffffffff81472aab>] [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] RSP: 0018:ffff88003daf1938 EFLAGS: 00010246
[ 303.020068] RAX: ffff88003db01400 RBX: ffffffffa00612a0 RCX: 0000000000000000
[ 303.020068] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003db01600
[ 303.020068] RBP: ffff88003daf1948 R08: 0000000000000000 R09: ffff88003db01400
[ 303.020068] R10: ffff88003fbe9fe8 R11: dead000000200200 R12: ffff88003d37c600
[ 303.020068] R13: 00000000fffffff1 R14: ffffffffa00612a0 R15: 00000000fffffff1
[ 303.020068] FS: 00007f1b3575a700(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 303.020068] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 303.020068] CR2: 0000000000000398 CR3: 000000003d2a9000 CR4: 00000000000006f0
[ 303.020068] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 303.020068] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 303.020068] Process tc (pid: 3138, threadinfo ffff88003daf0000, task ffff88003d407500)
[ 303.020068] Stack:
[ 303.020068] ffff88003e231000 ffff88003d6e1024 ffff88003daf19b8 ffffffff81474d68
[ 303.020068] 0000000000000001 0000000000000000 ffff88003d37c600 ffff88003daf1a08
[ 303.020068] 0073736572676e69 0000000000000000 ffff88003daf19b8 ffff88003d6e1010
[ 303.020068] Call Trace:
[ 303.020068] [<ffffffff81474d68>] qdisc_create+0x78/0x350
[ 303.020068] [<ffffffff81476849>] tc_modify_qdisc+0x339/0x590
[ 303.020068] [<ffffffff8146622f>] rtnetlink_rcv_msg+0x16f/0x280
[ 303.020068] [<ffffffff814660c0>] ? rtnetlink_rcv_msg+0x0/0x280
[ 303.020068] [<ffffffff8147d6d9>] netlink_rcv_skb+0xa9/0xd0
[ 303.020068] [<ffffffff814660b0>] rtnetlink_rcv+0x20/0x30
[ 303.020068] [<ffffffff8147ce85>] netlink_unicast+0x2c5/0x2e0
[ 303.020068] [<ffffffff8147e195>] netlink_sendmsg+0x245/0x360
[ 303.020068] [<ffffffff81444874>] sock_sendmsg+0xe4/0x110
[ 303.020068] [<ffffffff810c5e19>] ? find_get_page+0x19/0x90
[ 303.020068] [<ffffffff810c716a>] ? filemap_fault+0xca/0x4b0
[ 303.020068] [<ffffffff810c6cd5>] ? unlock_page+0x25/0x30
[ 303.020068] [<ffffffff81444b6d>] ? move_addr_to_kernel+0x5d/0x60
[ 303.020068] [<ffffffff814500bd>] ? verify_iovec+0x7d/0xf0
[ 303.020068] [<ffffffff814465b5>] sys_sendmsg+0x1e5/0x330
[ 303.020068] [<ffffffff8147b5c8>] ? netlink_table_ungrab+0x28/0x30
[ 303.020068] [<ffffffff81516fac>] ? do_page_fault+0x1dc/0x4c0
[ 303.020068] [<ffffffff81444abb>] ? move_addr_to_user+0x9b/0xb0
[ 303.020068] [<ffffffff8112463b>] ? alloc_fd+0x4b/0x140
[ 303.020068] [<ffffffff81446394>] ? sys_recvmsg+0x44/0x80
[ 303.020068] [<ffffffff81002eab>] system_call_fastpath+0x16/0x1b
[ 303.020068] Code: 8d 90 88 00 00 00 48 89 90 88 00 00 00 48 89 90 90 00 00 00 48 8b 53 28 48 89 10 48 8b 53 30 4c 89 60 68 48 89 50 08 49 8b 14 24 <48> 8b 92 98 03 00 00 65 ff 02 c7 40 40 01 00 00 00 5b 41 5c c9
[ 303.020068] RIP [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] RSP <ffff88003daf1938>
[ 303.020068] CR2: 0000000000000398
[ 303.135737] ---[ end trace 5b6e09a3328c82e4 ]---
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..8083c68 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
queue = kzalloc(sizeof(*queue), GFP_KERNEL);
if (!queue)
return NULL;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
netdev_init_one_queue(dev, queue, NULL);
queue->qdisc = &noop_qdisc;
queue->qdisc_sleeping = &noop_qdisc;
^ permalink raw reply related
* [PATCH 1/3] ifb: remove the useless debug stats
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
These debug stats are not exported, and become useless.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index ab9f675..4387525 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -42,16 +42,6 @@
struct ifb_private {
struct tasklet_struct ifb_tasklet;
int tasklet_pending;
- /* mostly debug stats leave in for now */
- unsigned long st_task_enter; /* tasklet entered */
- unsigned long st_txq_refl_try; /* transmit queue refill attempt */
- unsigned long st_rxq_enter; /* receive queue entered */
- unsigned long st_rx2tx_tran; /* receive to trasmit transfers */
- unsigned long st_rxq_notenter; /*receiveQ not entered, resched */
- unsigned long st_rx_frm_egr; /* received from egress path */
- unsigned long st_rx_frm_ing; /* received from ingress path */
- unsigned long st_rxq_check;
- unsigned long st_rxq_rsch;
struct sk_buff_head rq;
struct sk_buff_head tq;
};
@@ -73,19 +63,14 @@ static void ri_tasklet(unsigned long dev)
struct sk_buff *skb;
txq = netdev_get_tx_queue(_dev, 0);
- dp->st_task_enter++;
if ((skb = skb_peek(&dp->tq)) == NULL) {
- dp->st_txq_refl_try++;
if (__netif_tx_trylock(txq)) {
- dp->st_rxq_enter++;
while ((skb = skb_dequeue(&dp->rq)) != NULL) {
skb_queue_tail(&dp->tq, skb);
- dp->st_rx2tx_tran++;
}
__netif_tx_unlock(txq);
} else {
/* reschedule */
- dp->st_rxq_notenter++;
goto resched;
}
}
@@ -110,10 +95,8 @@ static void ri_tasklet(unsigned long dev)
skb->skb_iif = _dev->ifindex;
if (from & AT_EGRESS) {
- dp->st_rx_frm_egr++;
dev_queue_xmit(skb);
} else if (from & AT_INGRESS) {
- dp->st_rx_frm_ing++;
skb_pull(skb, skb->dev->hard_header_len);
netif_rx(skb);
} else
@@ -121,13 +104,11 @@ static void ri_tasklet(unsigned long dev)
}
if (__netif_tx_trylock(txq)) {
- dp->st_rxq_check++;
if ((skb = skb_peek(&dp->rq)) == NULL) {
dp->tasklet_pending = 0;
if (netif_queue_stopped(_dev))
netif_wake_queue(_dev);
} else {
- dp->st_rxq_rsch++;
__netif_tx_unlock(txq);
goto resched;
}
^ permalink raw reply related
* [PATCH 2/3] ifb: remove unused macro TX_TIMEOUT
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 4387525..d1e362a 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -36,8 +36,6 @@
#include <net/pkt_sched.h>
#include <net/net_namespace.h>
-#define TX_TIMEOUT (2*HZ)
-
#define TX_Q_LIMIT 32
struct ifb_private {
struct tasklet_struct ifb_tasklet;
^ permalink raw reply related
* [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
tq is only used in ri_tasklet, so we move it from ifb_private to a
stack variable of ri_tasklet.
skb_queue_splice_tail_init() is used instead of the open coded and slow
one.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
1 file changed, 12 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index d1e362a..cd6e90d 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -39,9 +39,7 @@
#define TX_Q_LIMIT 32
struct ifb_private {
struct tasklet_struct ifb_tasklet;
- int tasklet_pending;
struct sk_buff_head rq;
- struct sk_buff_head tq;
};
static int numifbs = 2;
@@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
static void ri_tasklet(unsigned long dev)
{
-
struct net_device *_dev = (struct net_device *)dev;
struct ifb_private *dp = netdev_priv(_dev);
struct net_device_stats *stats = &_dev->stats;
struct netdev_queue *txq;
struct sk_buff *skb;
+ struct sk_buff_head tq;
+ __skb_queue_head_init(&tq);
txq = netdev_get_tx_queue(_dev, 0);
- if ((skb = skb_peek(&dp->tq)) == NULL) {
- if (__netif_tx_trylock(txq)) {
- while ((skb = skb_dequeue(&dp->rq)) != NULL) {
- skb_queue_tail(&dp->tq, skb);
- }
- __netif_tx_unlock(txq);
- } else {
- /* reschedule */
- goto resched;
- }
+ if (!__netif_tx_trylock(txq)) {
+ tasklet_schedule(&dp->ifb_tasklet);
+ return;
}
+ skb_queue_splice_tail_init(&dp->rq, &tq);
+ if (netif_tx_queue_stopped(txq))
+ netif_tx_wake_queue(txq);
+ __netif_tx_unlock(txq);
- while ((skb = skb_dequeue(&dp->tq)) != NULL) {
+ while ((skb = __skb_dequeue(&tq)) != NULL) {
u32 from = G_TC_FROM(skb->tc_verd);
skb->tc_verd = 0;
@@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
rcu_read_unlock();
dev_kfree_skb(skb);
stats->tx_dropped++;
- break;
+ continue;
}
rcu_read_unlock();
skb->skb_iif = _dev->ifindex;
@@ -100,23 +96,6 @@ static void ri_tasklet(unsigned long dev)
} else
BUG();
}
-
- if (__netif_tx_trylock(txq)) {
- if ((skb = skb_peek(&dp->rq)) == NULL) {
- dp->tasklet_pending = 0;
- if (netif_queue_stopped(_dev))
- netif_wake_queue(_dev);
- } else {
- __netif_tx_unlock(txq);
- goto resched;
- }
- __netif_tx_unlock(txq);
- } else {
-resched:
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
- }
-
}
static const struct net_device_ops ifb_netdev_ops = {
@@ -162,10 +141,8 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
}
skb_queue_tail(&dp->rq, skb);
- if (!dp->tasklet_pending) {
- dp->tasklet_pending = 1;
+ if (skb_queue_len(&dp->rq) == 1)
tasklet_schedule(&dp->ifb_tasklet);
- }
return NETDEV_TX_OK;
}
@@ -177,7 +154,6 @@ static int ifb_close(struct net_device *dev)
tasklet_kill(&dp->ifb_tasklet);
netif_stop_queue(dev);
skb_queue_purge(&dp->rq);
- skb_queue_purge(&dp->tq);
return 0;
}
@@ -187,7 +163,6 @@ static int ifb_open(struct net_device *dev)
tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
skb_queue_head_init(&dp->rq);
- skb_queue_head_init(&dp->tq);
netif_start_queue(dev);
return 0;
^ permalink raw reply related
* Re: kernel panic with time-stamping in phy devices (monitor mode)
From: Richard Cochran @ 2010-12-04 7:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrew Watts, netdev, David Miller
In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
Date: Sat, 4 Dec 2010 08:55:04 +0100
From: Richard Cochran <richardcochran@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Watts <akwatts@ymail.com>, netdev@vger.kernel.org,
David Miller <davem@davemloft.net>
Subject: Re: kernel panic with time-stamping in phy devices (monitor mode)
Message-ID: <20101204075503.GA3490@riccoc20.at.omicron.at>
References: <252997.92320.qm@web111013.mail.gq1.yahoo.com>
<1291307884.2871.69.camel@edumazet-laptop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
User-Agent: Mutt/1.5.20 (2009-06-14)
Ugh, new kernel code with no users is already causing trouble!
On Thu, Dec 02, 2010 at 05:38:04PM +0100, Eric Dumazet wrote:
> Thanks for the report
>
> Please try following patch.
And thank you, Eric, for the quick patch.
Can this fix go into 2.6.37, please?
Thanks,
Richard
^ permalink raw reply
* Re: kernel panic with time-stamping in phy devices (monitor mode)
From: Eric Dumazet @ 2010-12-04 8:17 UTC (permalink / raw)
To: Richard Cochran; +Cc: Andrew Watts, netdev, David Miller
In-Reply-To: <20101204075750.GA3949@riccoc20.at.omicron.at>
Le samedi 04 décembre 2010 à 08:57 +0100, Richard Cochran a écrit :
> Date: Sat, 4 Dec 2010 08:55:04 +0100
> From: Richard Cochran <richardcochran@gmail.com>
> To: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Andrew Watts <akwatts@ymail.com>, netdev@vger.kernel.org,
> David Miller <davem@davemloft.net>
> Subject: Re: kernel panic with time-stamping in phy devices (monitor mode)
> Message-ID: <20101204075503.GA3490@riccoc20.at.omicron.at>
> References: <252997.92320.qm@web111013.mail.gq1.yahoo.com>
> <1291307884.2871.69.camel@edumazet-laptop>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
> User-Agent: Mutt/1.5.20 (2009-06-14)
>
> Ugh, new kernel code with no users is already causing trouble!
>
> On Thu, Dec 02, 2010 at 05:38:04PM +0100, Eric Dumazet wrote:
> > Thanks for the report
> >
> > Please try following patch.
>
> And thank you, Eric, for the quick patch.
>
> Can this fix go into 2.6.37, please?
Sure, I'll submit to David today, thanks !
^ permalink raw reply
* Re: [PATCH] net: init ingress queue
From: Eric Dumazet @ 2010-12-04 8:47 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291441558-3196-1-git-send-email-xiaosuo@gmail.com>
Le samedi 04 décembre 2010 à 13:45 +0800, Changli Gao a écrit :
> The dev field of ingress queue is forgot to initialized, then NULL
> pointer dereference happens in qdisc_alloc().
< deleted oops >
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> net/core/dev.c | 2 ++
> 1 file changed, 2 insertions(+)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..8083c68 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
> queue = kzalloc(sizeof(*queue), GFP_KERNEL);
> if (!queue)
> return NULL;
> + netdev_queue_numa_node_write(queue, -1);
> + queue->dev = dev;
> netdev_init_one_queue(dev, queue, NULL);
> queue->qdisc = &noop_qdisc;
> queue->qdisc_sleeping = &noop_qdisc;
Hi Changli, thanks for bug report and patch.
IMHO there is no point to include this long Oops report in Changelog for
a net-next-2.6 temporary problem, there wont be any bugzilla report.
Instead, you could say it is a followup patch for commits 1d24eb4815d1e0
and f2cd2d3e9b3ef960. Two or three lines with relevant information.
I suggest you submit following patch instead, as this seems cleaner to
me ?
(factorize the two inits in netdev_init_one_queue())
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..36e10b4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5113,7 +5113,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
{
unsigned int count = dev->num_tx_queues;
struct netdev_queue *tx;
- int i;
BUG_ON(count < 1);
@@ -5125,10 +5124,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
}
dev->_tx = tx;
- for (i = 0; i < count; i++) {
- netdev_queue_numa_node_write(&tx[i], -1);
- tx[i].dev = dev;
- }
return 0;
}
@@ -5140,6 +5135,8 @@ static void netdev_init_one_queue(struct net_device *dev,
spin_lock_init(&queue->_xmit_lock);
netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
queue->xmit_lock_owner = -1;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
}
static void netdev_init_queues(struct net_device *dev)
^ permalink raw reply related
* Re: [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 8:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291452427.2806.90.camel@edumazet-laptop>
On Sat, Dec 4, 2010 at 4:47 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le samedi 04 décembre 2010 à 13:45 +0800, Changli Gao a écrit :
>> The dev field of ingress queue is forgot to initialized, then NULL
>> pointer dereference happens in qdisc_alloc().
>
> < deleted oops >
>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> net/core/dev.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index cd24374..8083c68 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
>> queue = kzalloc(sizeof(*queue), GFP_KERNEL);
>> if (!queue)
>> return NULL;
>> + netdev_queue_numa_node_write(queue, -1);
>> + queue->dev = dev;
>> netdev_init_one_queue(dev, queue, NULL);
>> queue->qdisc = &noop_qdisc;
>> queue->qdisc_sleeping = &noop_qdisc;
>
> Hi Changli, thanks for bug report and patch.
>
> IMHO there is no point to include this long Oops report in Changelog for
> a net-next-2.6 temporary problem, there wont be any bugzilla report.
>
OK, Thanks.
> Instead, you could say it is a followup patch for commits 1d24eb4815d1e0
> and f2cd2d3e9b3ef960. Two or three lines with relevant information.
>
> I suggest you submit following patch instead, as this seems cleaner to
> me ?
>
> (factorize the two inits in netdev_init_one_queue())
>
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..36e10b4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5113,7 +5113,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> {
> unsigned int count = dev->num_tx_queues;
> struct netdev_queue *tx;
> - int i;
>
> BUG_ON(count < 1);
>
> @@ -5125,10 +5124,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> }
> dev->_tx = tx;
>
> - for (i = 0; i < count; i++) {
> - netdev_queue_numa_node_write(&tx[i], -1);
> - tx[i].dev = dev;
> - }
> return 0;
> }
>
> @@ -5140,6 +5135,8 @@ static void netdev_init_one_queue(struct net_device *dev,
> spin_lock_init(&queue->_xmit_lock);
> netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
> queue->xmit_lock_owner = -1;
> + netdev_queue_numa_node_write(queue, -1);
> + queue->dev = dev;
> }
>
> static void netdev_init_queues(struct net_device *dev)
>
>
>
I thought about it before submitting. I am not sure if there are some
users of txq->dev before netdev_init_one_queue().
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 1/2] vsprintf: Add %pMbt, bluetooth mac address
From: Michał Mirosław @ 2010-12-04 11:03 UTC (permalink / raw)
To: Joe Perches; +Cc: Marcel Holtmann, Gustavo F. Padovan, linux-kernel, netdev
In-Reply-To: <a33a0b00eeb29713a08d91156cbb2d816176f990.1291419007.git.joe@perches.com>
2010/12/4 Joe Perches <joe@perches.com>:
> Bluetooth output the MAC address in reverse order.
> Bluetooth memory order: 00 01 02 03 04 05 is output "05:04:03:02:01:00".
>
> This can save overall text when bluetooth is compiled in.
>
> Bluetooth currently uses a very slightly unsafe local function (batostr)
> to output these formatted addresses.
>
> Adding %pMbt allows the batostr function to be removed.
Just a nitpick:
You could call it %pMR, as in 'Reverse', so it sounds better when/if
some other subsystem uses it. It would also be a hint of what is this
doing instead of where it came from.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 12:31 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, Tom Herbert, Jiri Pirko, netdev, Changli Gao
The dev field of ingress queue is forgot to initialized, then NULL
pointer dereference happens in qdisc_alloc().
Move inits of tx queues to netif_alloc_netdev_queues().
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v2: factorize the two inits in netdev_init_queues() and move inits of
tx queues to netif_alloc_netdev_queues().
net/core/dev.c | 35 +++++++++++++----------------------
1 file changed, 13 insertions(+), 22 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..4a7fc32 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5109,11 +5109,21 @@ static int netif_alloc_rx_queues(struct net_device *dev)
}
#endif
+static void netdev_init_one_queue(struct net_device *dev,
+ struct netdev_queue *queue, void *_unused)
+{
+ /* Initialize queue lock */
+ spin_lock_init(&queue->_xmit_lock);
+ netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
+ queue->xmit_lock_owner = -1;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
+}
+
static int netif_alloc_netdev_queues(struct net_device *dev)
{
unsigned int count = dev->num_tx_queues;
struct netdev_queue *tx;
- int i;
BUG_ON(count < 1);
@@ -5125,27 +5135,10 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
}
dev->_tx = tx;
- for (i = 0; i < count; i++) {
- netdev_queue_numa_node_write(&tx[i], -1);
- tx[i].dev = dev;
- }
- return 0;
-}
-
-static void netdev_init_one_queue(struct net_device *dev,
- struct netdev_queue *queue,
- void *_unused)
-{
- /* Initialize queue lock */
- spin_lock_init(&queue->_xmit_lock);
- netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
- queue->xmit_lock_owner = -1;
-}
-
-static void netdev_init_queues(struct net_device *dev)
-{
netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
spin_lock_init(&dev->tx_global_lock);
+
+ return 0;
}
/**
@@ -5184,8 +5177,6 @@ int register_netdevice(struct net_device *dev)
dev->iflink = -1;
- netdev_init_queues(dev);
-
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
^ permalink raw reply related
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Jarek Poplawski @ 2010-12-04 13:15 UTC (permalink / raw)
To: Changli Gao; +Cc: jamal, netdev
In-Reply-To: <1291442121-3302-3-git-send-email-xiaosuo@gmail.com>
Changli Gao wrote:
> tq is only used in ri_tasklet, so we move it from ifb_private to a
> stack variable of ri_tasklet.
>
> skb_queue_splice_tail_init() is used instead of the open coded and slow
> one.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
> 1 file changed, 12 insertions(+), 37 deletions(-)
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index d1e362a..cd6e90d 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -39,9 +39,7 @@
> #define TX_Q_LIMIT 32
> struct ifb_private {
> struct tasklet_struct ifb_tasklet;
> - int tasklet_pending;
> struct sk_buff_head rq;
> - struct sk_buff_head tq;
> };
>
> static int numifbs = 2;
> @@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
>
> static void ri_tasklet(unsigned long dev)
> {
> -
> struct net_device *_dev = (struct net_device *)dev;
> struct ifb_private *dp = netdev_priv(_dev);
> struct net_device_stats *stats = &_dev->stats;
> struct netdev_queue *txq;
> struct sk_buff *skb;
> + struct sk_buff_head tq;
>
> + __skb_queue_head_init(&tq);
> txq = netdev_get_tx_queue(_dev, 0);
> - if ((skb = skb_peek(&dp->tq)) == NULL) {
> - if (__netif_tx_trylock(txq)) {
> - while ((skb = skb_dequeue(&dp->rq)) != NULL) {
> - skb_queue_tail(&dp->tq, skb);
> - }
> - __netif_tx_unlock(txq);
> - } else {
> - /* reschedule */
> - goto resched;
> - }
> + if (!__netif_tx_trylock(txq)) {
> + tasklet_schedule(&dp->ifb_tasklet);
> + return;
> }
> + skb_queue_splice_tail_init(&dp->rq, &tq);
> + if (netif_tx_queue_stopped(txq))
> + netif_tx_wake_queue(txq);
> + __netif_tx_unlock(txq);
>
> - while ((skb = skb_dequeue(&dp->tq)) != NULL) {
> + while ((skb = __skb_dequeue(&tq)) != NULL) {
> u32 from = G_TC_FROM(skb->tc_verd);
>
> skb->tc_verd = 0;
> @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
> rcu_read_unlock();
> dev_kfree_skb(skb);
> stats->tx_dropped++;
> - break;
> + continue;
IMHO this line is a bugfix and should be a separate patch (for stable).
The rest looks OK to me but you could probably skip locking of dp->rq
similarly to tq.
Jarek P.
> }
> rcu_read_unlock();
> skb->skb_iif = _dev->ifindex;
> @@ -100,23 +96,6 @@ static void ri_tasklet(unsigned long dev)
> } else
> BUG();
> }
> -
> - if (__netif_tx_trylock(txq)) {
> - if ((skb = skb_peek(&dp->rq)) == NULL) {
> - dp->tasklet_pending = 0;
> - if (netif_queue_stopped(_dev))
> - netif_wake_queue(_dev);
> - } else {
> - __netif_tx_unlock(txq);
> - goto resched;
> - }
> - __netif_tx_unlock(txq);
> - } else {
> -resched:
> - dp->tasklet_pending = 1;
> - tasklet_schedule(&dp->ifb_tasklet);
> - }
> -
> }
>
> static const struct net_device_ops ifb_netdev_ops = {
> @@ -162,10 +141,8 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
> }
>
> skb_queue_tail(&dp->rq, skb);
> - if (!dp->tasklet_pending) {
> - dp->tasklet_pending = 1;
> + if (skb_queue_len(&dp->rq) == 1)
> tasklet_schedule(&dp->ifb_tasklet);
> - }
>
> return NETDEV_TX_OK;
> }
> @@ -177,7 +154,6 @@ static int ifb_close(struct net_device *dev)
> tasklet_kill(&dp->ifb_tasklet);
> netif_stop_queue(dev);
> skb_queue_purge(&dp->rq);
> - skb_queue_purge(&dp->tq);
> return 0;
> }
>
> @@ -187,7 +163,6 @@ static int ifb_open(struct net_device *dev)
>
> tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
> skb_queue_head_init(&dp->rq);
> - skb_queue_head_init(&dp->tq);
> netif_start_queue(dev);
>
> return 0;
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 13:29 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: jamal, netdev
In-Reply-To: <4CFA3F01.20109@gmail.com>
On Sat, Dec 4, 2010 at 9:15 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> Changli Gao wrote:
>> tq is only used in ri_tasklet, so we move it from ifb_private to a
>> stack variable of ri_tasklet.
>>
>> skb_queue_splice_tail_init() is used instead of the open coded and slow
>> one.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
>> 1 file changed, 12 insertions(+), 37 deletions(-)
>> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
>> index d1e362a..cd6e90d 100644
>> --- a/drivers/net/ifb.c
>> +++ b/drivers/net/ifb.c
>> @@ -39,9 +39,7 @@
>> #define TX_Q_LIMIT 32
>> struct ifb_private {
>> struct tasklet_struct ifb_tasklet;
>> - int tasklet_pending;
>> struct sk_buff_head rq;
>> - struct sk_buff_head tq;
>> };
>>
>> static int numifbs = 2;
>> @@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
>>
>> static void ri_tasklet(unsigned long dev)
>> {
>> -
>> struct net_device *_dev = (struct net_device *)dev;
>> struct ifb_private *dp = netdev_priv(_dev);
>> struct net_device_stats *stats = &_dev->stats;
>> struct netdev_queue *txq;
>> struct sk_buff *skb;
>> + struct sk_buff_head tq;
>>
>> + __skb_queue_head_init(&tq);
>> txq = netdev_get_tx_queue(_dev, 0);
>> - if ((skb = skb_peek(&dp->tq)) == NULL) {
>> - if (__netif_tx_trylock(txq)) {
>> - while ((skb = skb_dequeue(&dp->rq)) != NULL) {
>> - skb_queue_tail(&dp->tq, skb);
>> - }
>> - __netif_tx_unlock(txq);
>> - } else {
>> - /* reschedule */
>> - goto resched;
>> - }
>> + if (!__netif_tx_trylock(txq)) {
>> + tasklet_schedule(&dp->ifb_tasklet);
>> + return;
>> }
>> + skb_queue_splice_tail_init(&dp->rq, &tq);
>> + if (netif_tx_queue_stopped(txq))
>> + netif_tx_wake_queue(txq);
>> + __netif_tx_unlock(txq);
>>
>> - while ((skb = skb_dequeue(&dp->tq)) != NULL) {
>> + while ((skb = __skb_dequeue(&tq)) != NULL) {
>> u32 from = G_TC_FROM(skb->tc_verd);
>>
>> skb->tc_verd = 0;
>> @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
>> rcu_read_unlock();
>> dev_kfree_skb(skb);
>> stats->tx_dropped++;
>> - break;
>> + continue;
>
> IMHO this line is a bugfix and should be a separate patch (for stable).
It sounds reasonable.
>
> The rest looks OK to me but you could probably skip locking of dp->rq
> similarly to tq.
>
OK. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] net: init ingress queue
From: Jarek Poplawski @ 2010-12-04 13:36 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Eric Dumazet, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291465901-3189-1-git-send-email-xiaosuo@gmail.com>
Changli Gao wrote:
> The dev field of ingress queue is forgot to initialized, then NULL
> pointer dereference happens in qdisc_alloc().
>
> Move inits of tx queues to netif_alloc_netdev_queues().
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v2: factorize the two inits in netdev_init_queues() and move inits of
> tx queues to netif_alloc_netdev_queues().
> net/core/dev.c | 35 +++++++++++++----------------------
> 1 file changed, 13 insertions(+), 22 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..4a7fc32 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
I guess you should mention it's net-next problem, because some people
might be unnecessarily concerned (why they don't have such an oops ;-)
Jarek P.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox