* [PATCH] net: phy: smsc: force all capable mode if the phy is started in powerdown mode
From: Philippe Reynes @ 2012-12-01 20:44 UTC (permalink / raw)
To: netdev, linux-kernel, davem
Cc: otavio, javier, jkosina, eric.jarrige, julien.boibessot,
thomas.petazzoni, Philippe Reynes
A SMSC PHY in power down mode can't be used.
If a SMSC PHY is in this mode in the config_init
stage, the mode "all capable" is set. So the PHY
could then be used.
Signed-off-by: Philippe Reynes <tremyfr@yahoo.fr>
---
drivers/net/phy/smsc.c | 27 ++++++++++++++++++++++++++-
include/linux/smscphy.h | 5 +++++
2 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 88e3991..63018d0 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,7 +43,32 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
static int smsc_phy_config_init(struct phy_device *phydev)
{
- int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ /*
+ * If the SMSC PHY is in power down mode, then set it
+ * in all capable mode before using it.
+ */
+ int rc = phy_read(phydev, MII_LAN83C185_SPECIAL_MODES);
+ if (rc < 0)
+ return rc;
+
+ if ((rc & MII_LAN83C185_MODE_MASK) == MII_LAN83C185_MODE_POWERDOWN) {
+ int timeout = 50000;
+
+ /* set "all capable" mode and reset the phy */
+ rc |= MII_LAN83C185_MODE_ALL;
+ phy_write(phydev, MII_LAN83C185_SPECIAL_MODES, rc);
+ phy_write(phydev, MII_BMCR, BMCR_RESET);
+
+ /* wait end of reset (max 500 ms) */
+ do {
+ udelay(10);
+ if (timeout-- == 0)
+ return -1;
+ rc = phy_read(phydev, MII_BMCR);
+ } while (rc & BMCR_RESET);
+ }
+
+ rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index ce718cb..f4bf16e 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -4,6 +4,7 @@
#define MII_LAN83C185_ISF 29 /* Interrupt Source Flags */
#define MII_LAN83C185_IM 30 /* Interrupt Mask */
#define MII_LAN83C185_CTRL_STATUS 17 /* Mode/Status Register */
+#define MII_LAN83C185_SPECIAL_MODES 18 /* Special Modes Register */
#define MII_LAN83C185_ISF_INT1 (1<<1) /* Auto-Negotiation Page Received */
#define MII_LAN83C185_ISF_INT2 (1<<2) /* Parallel Detection Fault */
@@ -22,4 +23,8 @@
#define MII_LAN83C185_EDPWRDOWN (1 << 13) /* EDPWRDOWN */
#define MII_LAN83C185_ENERGYON (1 << 1) /* ENERGYON */
+#define MII_LAN83C185_MODE_MASK 0xE0
+#define MII_LAN83C185_MODE_POWERDOWN 0xC0 /* Power Down mode */
+#define MII_LAN83C185_MODE_ALL 0xE0 /* All capable mode */
+
#endif /* __LINUX_SMSCPHY_H__ */
--
1.7.4.4
^ permalink raw reply related
* Re: sky2: Correct mistakenly switched read/write sequence
From: Lino Sanfilippo @ 2012-12-01 20:06 UTC (permalink / raw)
To: David Miller; +Cc: LinoSanfilippo, shemminger, mlindner, netdev
In-Reply-To: <20121201.122701.2065141028797970034.davem@davemloft.net>
On Sat, Dec 01, 2012 at 12:27:01PM -0500, David Miller wrote:
>
> > In sky2_all_down() the order of the read()/write() access to B0_IMSK seems to
> > be mistakenly switched. The original intention was obviously to avoid PCI write
> > posting.
> > This patch fixes the order.
> >
> > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
>
> I would say that no such intention exists at all.
>
> The read is there because a long time ago the result as used
> to compute an 'imask' variable.
>
> Please see commit:
>
> commit d72ff8fa7f8b344382963721f842256825c4660b
I see. Indeed I was not aware of the history that led to the recent code, so
thanks for pointing that out.
However, since Stephen thinks the patch is useful nevertheless:
@Stephen please feel free to adjust the commit message as you think would make
the most sense.
Regards,
Lino
^ permalink raw reply
* Re: [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Simon Wunderlich @ 2012-12-01 20:01 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Simon Wunderlich,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1354391074.20109.527.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]
On Sat, Dec 01, 2012 at 11:44:34AM -0800, Eric Dumazet wrote:
> On Sat, 2012-12-01 at 20:04 +0100, Sven Eckelmann wrote:
> > On Saturday 01 December 2012 10:36:12 Eric Dumazet wrote:
> >
> > > > diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> > > > index c5c0593..c122782 100644
> > > > --- a/net/bridge/br_sysfs_br.c
> > > > +++ b/net/bridge/br_sysfs_br.c
> > > > @@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
> > > >
> > > > if (!rtnl_trylock())
> > > >
> > > > return restart_syscall();
> > > >
> > > > br_stp_set_enabled(br, val);
> > > >
> > > > - rtnl_unlock();
> > > > + __rtnl_unlock();
> > > >
> > > > return len;
> > > >
> > > > }
> > >
> > > I have no idea of why you believe there is a problem here.
> > >
> > > Could you explain how net_todo_list could be not empty ?
> > >
> > > As long as no device is unregistered between
> > > rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
> >
> > I am not sure what "here" means for your. At least batman-adv tries to
> > unregister a device -> problem [1]. I will not make any judgements about the
> > other uses in the kernel/other parts patched by Simon.
> >
>
> I was reacting to the change in net/bridge/br_sysfs_br.c
>
> rtnl_trylock() could set a boolean flag to explicitly WARN_ON()
> in case we try to unregister a device.
Well, I'm not sure if this can happen in the bridge code, but from looking at the
code it doesn't appear to be impossible. It would be better to be sure that it can't
deadlock IMHO.
(Although doing unlock/lock/unlock in an unlock function is also a little "uncommon").
Cheers,
Simon
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Sven Eckelmann @ 2012-12-01 19:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, Simon Wunderlich
In-Reply-To: <1354391074.20109.527.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]
On Saturday 01 December 2012 11:44:34 Eric Dumazet wrote:
[...]
> > > I have no idea of why you believe there is a problem here.
> > >
> > > Could you explain how net_todo_list could be not empty ?
> > >
> > > As long as no device is unregistered between
> > > rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
> >
> > I am not sure what "here" means for your. At least batman-adv tries to
> > unregister a device -> problem [1]. I will not make any judgements about
> > the other uses in the kernel/other parts patched by Simon.
>
> I was reacting to the change in net/bridge/br_sysfs_br.c
Yes, in this context it makes more sense.
> rtnl_trylock() could set a boolean flag to explicitly WARN_ON()
> in case we try to unregister a device.
Sounds interesting.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Re: [B.A.T.M.A.N.] [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Eric Dumazet @ 2012-12-01 19:44 UTC (permalink / raw)
To: Sven Eckelmann
Cc: b.a.t.m.a.n, Simon Wunderlich, netdev, davem, Simon Wunderlich
In-Reply-To: <10264267.HX5FJDguj3@sven-laptop.home.narfation.org>
On Sat, 2012-12-01 at 20:04 +0100, Sven Eckelmann wrote:
> On Saturday 01 December 2012 10:36:12 Eric Dumazet wrote:
>
> > > diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> > > index c5c0593..c122782 100644
> > > --- a/net/bridge/br_sysfs_br.c
> > > +++ b/net/bridge/br_sysfs_br.c
> > > @@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
> > >
> > > if (!rtnl_trylock())
> > >
> > > return restart_syscall();
> > >
> > > br_stp_set_enabled(br, val);
> > >
> > > - rtnl_unlock();
> > > + __rtnl_unlock();
> > >
> > > return len;
> > >
> > > }
> >
> > I have no idea of why you believe there is a problem here.
> >
> > Could you explain how net_todo_list could be not empty ?
> >
> > As long as no device is unregistered between
> > rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
>
> I am not sure what "here" means for your. At least batman-adv tries to
> unregister a device -> problem [1]. I will not make any judgements about the
> other uses in the kernel/other parts patched by Simon.
>
I was reacting to the change in net/bridge/br_sysfs_br.c
rtnl_trylock() could set a boolean flag to explicitly WARN_ON()
in case we try to unregister a device.
^ permalink raw reply
* Re: GRO + splice panics in 3.7.0-rc5
From: Willy Tarreau @ 2012-12-01 19:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1353023344.10798.8.camel@edumazet-glaptop>
Hi Eric,
On Thu, Nov 15, 2012 at 03:49:04PM -0800, Eric Dumazet wrote:
> On Thu, 2012-11-15 at 23:28 +0100, Willy Tarreau wrote:
> > Hello,
> >
> > I was just about to make a quick comparison between LRO and GRO in
> > 3.7.0-rc5 to see if LRO still had the big advantage I've always observed,
> > but I failed the test because as soon as I enable LRO + splice, the kernel
> > panics and reboots.
> >
> > I could not yet manage to catch the panic output, I could just reliably
> > reproduce it, it crashes instantly.
> >
> > All I can say at the moment is the following :
> > - test consist in forwarding HTTP traffic between two NICs via haproxy
> > - driver used was myri10ge
> > - LRO + recv+send : OK
> > - LRO + splice : OK
> > - GRO + recv+send : OK
> > - GRO + splice : panic
> > - no such problem was observed in 3.6.6 so I think this is a recent
> > regression.
> >
> > I'll go back digging for more information, but as I'm used to often see
> > Eric suggest the right candidates for reverting, I wanted to report the
> > issue here in case there are easy ones to try first.
>
> Hi Willy
>
> Nothing particular comes to mind, there were a lot of recent changes
> that could trigger this kind of bug.
>
> A stack trace would be useful of course ;)
First, sorry for the long delay, I would really have liked to debug
this earlier.
I could finally bisect the regression to this commit :
69b08f62e17439ee3d436faf0b9a7ca6fffb78db is the first bad commit
commit 69b08f62e17439ee3d436faf0b9a7ca6fffb78db
Author: Eric Dumazet <edumazet@google.com>
Date: Wed Sep 26 06:46:57 2012 +0000
net: use bigger pages in __netdev_alloc_frag
The regression is still present in latest git (7c17e486), and goes away
if I revert this patch.
I get this trace both on my 64-bit 10Gig machines with a myri10ge NIC,
and on my 32-bit eeepc with an atl1c NIC at 100 Mbps, so this is not
related to the driver nor the architecture.
The trace looks like this on my eeepc, it crashes in tcp_sendpage()
(more precisely in do_tcp_sendpages() which was inlined in the former) :
BUG: unable to handle kernel NULL pointer dereference at 00000b50
IP: [<c13fdf9c>] tcp_sendpage+0x28c/0x5d0
*pde = 00000000
Oops: 0000 [#1] SMP
Modules linked in: snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss uvcvideo videobuf2_core videobuf2_vmalloc videobuf2_memops ath9k mac80211 snd_hda_codec_realtek microcode ath9k_common psmouse ath9k_hw serio_raw uhci_hcd ath sg cfg80211 snd_hda_intel lpc_ich ehci_hcd mfd_core snd_hda_codec atl1c snd_hwdep rtc_cmos snd_pcm snd_timer snd snd_page_alloc eeepc_laptop sparse_keymap evdev
Pid: 2880, comm: haproxy Not tainted 3.7.0-rc7-eeepc #1 ASUSTeK Computer INC. 1005HA/1005HA
EIP: 0060:[<c13fdf9c>] EFLAGS: 00210246 CPU: 0
EIP is at tcp_sendpage+0x28c/0x5d0
EAX: c16a4138 EBX: f5202500 ECX: 00000b50 EDX: f5a72f68
ESI: f5333cc0 EDI: 000005a8 EBP: f55b7e34 ESP: f55b7df0
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
CR0: 8005003b CR2: 00000b50 CR3: 35556000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process haproxy (pid: 2880, ti=f55b6000 task=f62060c0 task.ti=f55b6000)
Stack:
00000000 00000002 00000000 00000582 02000000 f52025a0 000005a8 00000002
00000b50 00001582 00000000 f6eb3900 00000b50 00000000 f5202500 c13fdd10
00000040 f55b7e5c c14201c7 000005a8 00000040 f55b7ecc f6eb3900 f5fc1680
Call Trace:
[<c13fdd10>] ? sk_stream_alloc_skb+0xf0/0xf0
[<c14201c7>] inet_sendpage+0x57/0xc0
[<c1420170>] ? inet_dgram_connect+0x80/0x80
[<c13b6d99>] kernel_sendpage+0x29/0x50
[<c13b6de8>] sock_sendpage+0x28/0x30
[<c13b6dc0>] ? kernel_sendpage+0x50/0x50
[<c10e55b5>] pipe_to_sendpage+0x65/0x80
[<c10e5647>] splice_from_pipe_feed+0x77/0x150
[<c10e5550>] ? splice_from_pipe_begin+0x10/0x10
[<c10e5a84>] __splice_from_pipe+0x54/0x60
[<c10e5550>] ? splice_from_pipe_begin+0x10/0x10
[<c10e7190>] splice_from_pipe+0x50/0x70
[<c10e7200>] ? default_file_splice_write+0x50/0x50
[<c10e7222>] generic_splice_sendpage+0x22/0x30
[<c10e5550>] ? splice_from_pipe_begin+0x10/0x10
[<c10e5af6>] do_splice_from+0x66/0x90
[<c10e787f>] sys_splice+0x42f/0x480
[<c14671cc>] syscall_call+0x7/0xb
[<c1460000>] ? blk_iopoll_cpu_notify+0x2c/0x5d
Code: 7d 08 0f 47 7d 08 39 f8 0f 4e f8 8b 43 1c 8b 40 60 85 c0 74 09 39 7b 64 0f 8c 21 01 00 00 80 7d ce 00 0f 85 ef 00 00 00 8b 4d dc <8b> 01 f6 c4 80 0f 85 0e 03 00 00 8b 45 dc f0 ff 40 10 8b 55 d8
EIP: [<c13fdf9c>] tcp_sendpage+0x28c/0x5d0 SS:ESP 0068:f55b7df0
CR2: 0000000000000b50
---[ end trace a07522433caf9655 ]---
In order to reproduce it, I'm just running haproxy with TCP splicing
enabled. It simply forwards data from one TCP socket to another one.
In case this matters, here's what the sequence looks like when it works
(just relevant syscalls). fd 7 = client retrieving data, fd 8 = server
providing data :
socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 8
fcntl64(8, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
setsockopt(8, SOL_TCP, TCP_NODELAY, [1], 4) = 0
connect(8, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("10.8.3.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
send(8, "GET /?s=100k HTTP/1.1\r\nConnection"..., 120, MSG_DONTWAIT|MSG_NOSIGNAL) = 120
pipe([9, 10]) = 0
splice(0x8, 0, 0xa, 0, 0x40000000, 0x3) = 5792
splice(0x8, 0, 0xa, 0, 0x40000000, 0x3) = 1448
splice(0x8, 0, 0xa, 0, 0x40000000, 0x3) = -1 EAGAIN (Resource temporarily unavailable)
splice(0x9, 0, 0x7, 0, 0x1c48, 0x3) = 7240
And the splice() block above repeats over and over with varying sizes.
It's worth mentionning that the issue does not happend when recv/send are
used instead of splice(). Changing the pipe size using F_SETPIPE_SZ does
not change the behaviour either (was using 1 MB when it first appeared and
thought it was the cause).
I managed to catch the oops with strace running on the process. It crashed
on the first splice() call on the sending side and confirms the stack trace :
pipe([9, 10]) = 0
splice(0x8, 0, 0xa, 0, 0x40000000, 0x3) = 10136
splice(0x9, 0, 0x7, 0, 0x2798, 0x3) ---> never completes, oops here.
Are there any additional information I can provide to help debug this ?
Seeing what the patch does, I don't think it's the direct responsible
but that it triggers an existing bug somewhere else. However I don't
know how to try to trigger the same bug without the patch.
The corresponding disassembled code looks like this :
1ee3: 0f 47 7d 08 cmova 0x8(%ebp),%edi
1ee7: 39 f8 cmp %edi,%eax
1ee9: 0f 4e f8 cmovle %eax,%edi
1eec: 8b 43 1c mov 0x1c(%ebx),%eax
1eef: 8b 40 60 mov 0x60(%eax),%eax
1ef2: 85 c0 test %eax,%eax
1ef4: 74 09 je 1eff <tcp_sendpage+0x27f>
1ef6: 39 7b 64 cmp %edi,0x64(%ebx)
1ef9: 0f 8c 21 01 00 00 jl 2020 <tcp_sendpage+0x3a0>
1eff: 80 7d ce 00 cmpb $0x0,-0x32(%ebp)
1f03: 0f 85 ef 00 00 00 jne 1ff8 <tcp_sendpage+0x378>
1f09: 8b 4d dc mov -0x24(%ebp),%ecx
--> 1f0c: 8b 01 mov (%ecx),%eax
1f0e: f6 c4 80 test $0x80,%ah
1f11: 0f 85 0b 03 00 00 jne 2222 <tcp_sendpage+0x5a2>
1f17: 8b 45 dc mov -0x24(%ebp),%eax
1f1a: f0 ff 40 10 lock incl 0x10(%eax)
1f1e: 8b 55 d8 mov -0x28(%ebp),%edx
1f21: 8b 4d dc mov -0x24(%ebp),%ecx
1f24: 8d 04 d5 20 00 00 00 lea 0x20(,%edx,8),%eax
1f2b: 03 86 90 00 00 00 add 0x90(%esi),%eax
This seems to be at the beginning of this block, at new_segment :
866 int size = min_t(size_t, psize, PAGE_SIZE - offset);
867 bool can_coalesce;
868
869 if (!tcp_send_head(sk) || (copy = size_goal - skb->len) <= 0) {
870 new_segment:
871 if (!sk_stream_memory_free(sk))
872 goto wait_for_sndbuf;
873
I'm trying to add some debug code.
Regards,
Willy
^ permalink raw reply
* Re: Re: [B.A.T.M.A.N.] [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Sven Eckelmann @ 2012-12-01 19:04 UTC (permalink / raw)
To: b.a.t.m.a.n
Cc: Eric Dumazet, Simon Wunderlich, netdev, davem, Simon Wunderlich
In-Reply-To: <1354386972.20109.523.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]
On Saturday 01 December 2012 10:36:12 Eric Dumazet wrote:
[...]
> > diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
> > index 66518c7..41b74aa 100644
> > --- a/net/batman-adv/sysfs.c
> > +++ b/net/batman-adv/sysfs.c
> > @@ -635,7 +635,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject
> > *kobj,>
> > ret = batadv_hardif_enable_interface(hard_iface, buff);
> >
> > unlock:
> > - rtnl_unlock();
> > + __rtnl_unlock();
> >
> > out:
> > batadv_hardif_free_ref(hard_iface);
> > return ret;
> >
> > diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> > index c5c0593..c122782 100644
> > --- a/net/bridge/br_sysfs_br.c
> > +++ b/net/bridge/br_sysfs_br.c
> > @@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
> >
> > if (!rtnl_trylock())
> >
> > return restart_syscall();
> >
> > br_stp_set_enabled(br, val);
> >
> > - rtnl_unlock();
> > + __rtnl_unlock();
> >
> > return len;
> >
> > }
>
> I have no idea of why you believe there is a problem here.
>
> Could you explain how net_todo_list could be not empty ?
>
> As long as no device is unregistered between
> rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
I am not sure what "here" means for your. At least batman-adv tries to
unregister a device -> problem [1]. I will not make any judgements about the
other uses in the kernel/other parts patched by Simon.
Kind regards,
Sven
[1] http://article.gmane.org/gmane.linux.kernel/1392295
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [net-next:master 100/106] net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
From: Eric Dumazet @ 2012-12-01 18:46 UTC (permalink / raw)
To: kbuild test robot; +Cc: Eric Dumazet, netdev
In-Reply-To: <50ba4a7c.o5mv0SK27e0dxcwD%fengguang.wu@intel.com>
On Sun, 2012-12-02 at 02:20 +0800, kbuild test robot wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> head: 577b981714b0b3530817569bf705bd74881efc83
> commit: ce43b03e8889475817d427b1f3724c7e294b76eb [100/106] net: move inet_dport/inet_num in sock_common
>
>
> sparse warnings:
>
> + net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
> net/ipv4/inet_hashtables.c:246:29: sparse: restricted __portpair degrades to integer
> net/ipv4/inet_hashtables.c:267:21: sparse: restricted __portpair degrades to integer
> net/ipv4/inet_hashtables.c:274:29: sparse: restricted __portpair degrades to integer
> net/ipv4/inet_hashtables.c:326:21: sparse: restricted __portpair degrades to integer
> net/ipv4/inet_hashtables.c:341:21: sparse: restricted __portpair degrades to integer
> --
> + net/ipv6/inet6_hashtables.c:92:21: sparse: restricted __portpair degrades to integer
> net/ipv6/inet6_hashtables.c:95:29: sparse: restricted __portpair degrades to integer
> net/ipv6/inet6_hashtables.c:111:21: sparse: restricted __portpair degrades to integer
> net/ipv6/inet6_hashtables.c:117:29: sparse: restricted __portpair degrades to integer
> net/ipv6/inet6_hashtables.c:248:21: sparse: restricted __portpair degrades to integer
> net/ipv6/inet6_hashtables.c:263:21: sparse: restricted __portpair degrades to integer
>
> vim +242 net/ipv4/inet_hashtables.c
I'll send a fix, thanks for the report.
^ permalink raw reply
* [PATCH net-next] sundance: Enable WoL support
From: Denis Kirjanov @ 2012-12-01 18:39 UTC (permalink / raw)
To: netdev; +Cc: davem, kda
In-Reply-To: <Denis Kirjanov <kda@linux-powerpc.org>
Enable WoL support.
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
drivers/net/ethernet/dlink/sundance.c | 81 ++++++++++++++++++++++++++++++++-
1 files changed, 80 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/dlink/sundance.c b/drivers/net/ethernet/dlink/sundance.c
index 3b83588..bbeb4c7 100644
--- a/drivers/net/ethernet/dlink/sundance.c
+++ b/drivers/net/ethernet/dlink/sundance.c
@@ -259,6 +259,7 @@ enum alta_offsets {
EECtrl = 0x36,
FlashAddr = 0x40,
FlashData = 0x44,
+ WakeEvent = 0x45,
TxStatus = 0x46,
TxFrameId = 0x47,
DownCounter = 0x18,
@@ -333,6 +334,14 @@ enum mac_ctrl1_bits {
RxEnable=0x0800, RxDisable=0x1000, RxEnabled=0x2000,
};
+/* Bits in WakeEvent register. */
+enum wake_event_bits {
+ WakePktEnable = 0x01,
+ MagicPktEnable = 0x02,
+ LinkEventEnable = 0x04,
+ WolEnable = 0x80,
+};
+
/* The Rx and Tx buffer descriptors. */
/* Note that using only 32 bit fields simplifies conversion to big-endian
architectures. */
@@ -392,6 +401,7 @@ struct netdev_private {
unsigned int default_port:4; /* Last dev->if_port value. */
unsigned int an_enable:1;
unsigned int speed;
+ unsigned int wol_enabled:1; /* Wake on LAN enabled */
struct tasklet_struct rx_tasklet;
struct tasklet_struct tx_tasklet;
int budget;
@@ -829,7 +839,7 @@ static int netdev_open(struct net_device *dev)
unsigned long flags;
int i;
- /* Do we need to reset the chip??? */
+ sundance_reset(dev, 0x00ff << 16);
i = request_irq(irq, intr_handler, IRQF_SHARED, dev->name, dev);
if (i)
@@ -877,6 +887,10 @@ static int netdev_open(struct net_device *dev)
iowrite16 (StatsEnable | RxEnable | TxEnable, ioaddr + MACCtrl1);
+ /* Disable Wol */
+ iowrite8(ioread8(ioaddr + WakeEvent) | 0x00, ioaddr + WakeEvent);
+ np->wol_enabled = 0;
+
if (netif_msg_ifup(np))
printk(KERN_DEBUG "%s: Done netdev_open(), status: Rx %x Tx %x "
"MAC Control %x, %4.4x %4.4x.\n",
@@ -1715,6 +1729,60 @@ static void get_ethtool_stats(struct net_device *dev,
data[i++] = np->xstats.rx_mcasts;
}
+#ifdef CONFIG_PM
+
+static void sundance_get_wol(struct net_device *dev,
+ struct ethtool_wolinfo *wol)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
+ u8 wol_bits;
+
+ wol->wolopts = 0;
+
+ wol->supported = (WAKE_PHY | WAKE_MAGIC);
+ if (!np->wol_enabled)
+ return;
+
+ wol_bits = ioread8(ioaddr + WakeEvent);
+ if (wol_bits & MagicPktEnable)
+ wol->wolopts |= WAKE_MAGIC;
+ if (wol_bits & LinkEventEnable)
+ wol->wolopts |= WAKE_PHY;
+}
+
+static int sundance_set_wol(struct net_device *dev,
+ struct ethtool_wolinfo *wol)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
+ u8 wol_bits;
+
+ if (!device_can_wakeup(&np->pci_dev->dev))
+ return -EOPNOTSUPP;
+
+ np->wol_enabled = !!(wol->wolopts);
+ wol_bits = ioread8(ioaddr + WakeEvent);
+ wol_bits &= ~(WakePktEnable | MagicPktEnable |
+ LinkEventEnable | WolEnable);
+
+ if (np->wol_enabled) {
+ if (wol->wolopts & WAKE_MAGIC)
+ wol_bits |= (MagicPktEnable | WolEnable);
+ if (wol->wolopts & WAKE_PHY)
+ wol_bits |= (LinkEventEnable | WolEnable);
+ }
+ iowrite8(wol_bits, ioaddr + WakeEvent);
+
+ device_set_wakeup_enable(&np->pci_dev->dev, np->wol_enabled);
+
+ return 0;
+}
+#else
+#define sundance_get_wol NULL
+#define sundance_set_wol NULL
+#endif /* CONFIG_PM */
+
static const struct ethtool_ops ethtool_ops = {
.begin = check_if_running,
.get_drvinfo = get_drvinfo,
@@ -1722,6 +1790,8 @@ static const struct ethtool_ops ethtool_ops = {
.set_settings = set_settings,
.nway_reset = nway_reset,
.get_link = get_link,
+ .get_wol = sundance_get_wol,
+ .set_wol = sundance_set_wol,
.get_msglevel = get_msglevel,
.set_msglevel = set_msglevel,
.get_strings = get_strings,
@@ -1867,6 +1937,8 @@ static void __devexit sundance_remove1 (struct pci_dev *pdev)
static int sundance_suspend(struct pci_dev *pci_dev, pm_message_t state)
{
struct net_device *dev = pci_get_drvdata(pci_dev);
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->base;
if (!netif_running(dev))
return 0;
@@ -1875,6 +1947,12 @@ static int sundance_suspend(struct pci_dev *pci_dev, pm_message_t state)
netif_device_detach(dev);
pci_save_state(pci_dev);
+ if (np->wol_enabled) {
+ iowrite8(AcceptBroadcast | AcceptMyPhys, ioaddr + RxMode);
+ iowrite16(RxEnable, ioaddr + MACCtrl1);
+ }
+ pci_enable_wake(pci_dev, pci_choose_state(pci_dev, state),
+ np->wol_enabled);
pci_set_power_state(pci_dev, pci_choose_state(pci_dev, state));
return 0;
@@ -1890,6 +1968,7 @@ static int sundance_resume(struct pci_dev *pci_dev)
pci_set_power_state(pci_dev, PCI_D0);
pci_restore_state(pci_dev);
+ pci_enable_wake(pci_dev, PCI_D0, 0);
err = netdev_open(dev);
if (err) {
--
1.7.3.4
^ permalink raw reply related
* Re: sky2: Correct mistakenly switched read/write sequence
From: Stephen Hemminger @ 2012-12-01 18:36 UTC (permalink / raw)
To: Lino Sanfilippo; +Cc: mlindner, davem, netdev
In-Reply-To: <20121201124239.GB3914@neptun>
On Sat, 1 Dec 2012 13:42:39 +0100
Lino Sanfilippo <LinoSanfilippo@gmx.de> wrote:
> In sky2_all_down() the order of the read()/write() access to B0_IMSK seems to
> be mistakenly switched. The original intention was obviously to avoid PCI write
> posting.
> This patch fixes the order.
>
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
You are both right. David is correct in that the original code here
was quite different and was doing save/restore of irq mask.
That changed as driver evolved to only bring up IRQ if device was
brought up. At which point the read of irq mask was a left over call.
Lino is correct, it makes sense to do read after write to ensure PCI posting.
So I agree with the patch, but would like the commit message changed
slightly.
^ permalink raw reply
* Re: [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Eric Dumazet @ 2012-12-01 18:36 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: davem, netdev, b.a.t.m.a.n, Simon Wunderlich
In-Reply-To: <1354382991-31350-1-git-send-email-siwu@hrz.tu-chemnitz.de>
On Sat, 2012-12-01 at 18:29 +0100, Simon Wunderlich wrote:
> If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
> may destroy this attempt because it first unlocks rtnl_mutex but may
> lock it again later. The callgraph roughly looks like:
>
> rtnl_unlock()
> netdev_run_todo()
> __rtnl_unlock()
> netdev_wait_allrefs()
> rtnl_lock()
> ...
> __rtnl_unlock()
>
> There are two users which have possible deadlocks and are fixed in this
> patch: batman-adv and bridge. Their problematic access pattern is the same:
>
> notifier_call handler:
> * holds rtnl lock when called
> * calls sysfs code at some point (acquiring sysfs locks)
>
> sysfs code:
> * holds sysfs lock when called
> * calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
> implicitly
>
> Fix this by exporting __rtnl_unlock() to just unlock the mutex without
> implicitly locking again.
>
> Reported-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>
> ---
> Of course, an alternative would be to not lock again after unlocking
> within rtnl_unlock(), or put the todo handling into the locked section.
> I'm not familiar enough with this code to know what would be best.
>
> There are others using rtnl_trylock(), but I'm not sure if they
> are affected.
> ---
> net/batman-adv/sysfs.c | 2 +-
> net/bridge/br_sysfs_br.c | 2 +-
> net/bridge/br_sysfs_if.c | 2 +-
> net/core/rtnetlink.c | 1 +
> 4 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
> index 66518c7..41b74aa 100644
> --- a/net/batman-adv/sysfs.c
> +++ b/net/batman-adv/sysfs.c
> @@ -635,7 +635,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
> ret = batadv_hardif_enable_interface(hard_iface, buff);
>
> unlock:
> - rtnl_unlock();
> + __rtnl_unlock();
> out:
> batadv_hardif_free_ref(hard_iface);
> return ret;
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index c5c0593..c122782 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
> if (!rtnl_trylock())
> return restart_syscall();
> br_stp_set_enabled(br, val);
> - rtnl_unlock();
> + __rtnl_unlock();
>
> return len;
> }
I have no idea of why you believe there is a problem here.
Could you explain how net_todo_list could be not empty ?
As long as no device is unregistered between
rtnl_trylock()/rtnl_unlock(), there is no possible deadlock.
^ permalink raw reply
* [net-next:master 100/106] net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
From: kbuild test robot @ 2012-12-01 18:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 577b981714b0b3530817569bf705bd74881efc83
commit: ce43b03e8889475817d427b1f3724c7e294b76eb [100/106] net: move inet_dport/inet_num in sock_common
sparse warnings:
+ net/ipv4/inet_hashtables.c:242:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:246:29: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:267:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:274:29: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:326:21: sparse: restricted __portpair degrades to integer
net/ipv4/inet_hashtables.c:341:21: sparse: restricted __portpair degrades to integer
--
+ net/ipv6/inet6_hashtables.c:92:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:95:29: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:111:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:117:29: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:248:21: sparse: restricted __portpair degrades to integer
net/ipv6/inet6_hashtables.c:263:21: sparse: restricted __portpair degrades to integer
vim +242 net/ipv4/inet_hashtables.c
77a5ba55 Pavel Emelyanov 2007-12-20 226 INET_ADDR_COOKIE(acookie, saddr, daddr)
77a5ba55 Pavel Emelyanov 2007-12-20 227 const __portpair ports = INET_COMBINED_PORTS(sport, hnum);
77a5ba55 Pavel Emelyanov 2007-12-20 228 struct sock *sk;
3ab5aee7 Eric Dumazet 2008-11-16 229 const struct hlist_nulls_node *node;
77a5ba55 Pavel Emelyanov 2007-12-20 230 /* Optimize here for direct hit, only listening connections can
77a5ba55 Pavel Emelyanov 2007-12-20 231 * have wildcards anyways.
77a5ba55 Pavel Emelyanov 2007-12-20 232 */
9f26b3ad Pavel Emelyanov 2008-06-16 233 unsigned int hash = inet_ehashfn(net, daddr, hnum, saddr, sport);
f373b53b Eric Dumazet 2009-10-09 234 unsigned int slot = hash & hashinfo->ehash_mask;
3ab5aee7 Eric Dumazet 2008-11-16 235 struct inet_ehash_bucket *head = &hashinfo->ehash[slot];
77a5ba55 Pavel Emelyanov 2007-12-20 236
3ab5aee7 Eric Dumazet 2008-11-16 237 rcu_read_lock();
3ab5aee7 Eric Dumazet 2008-11-16 238 begin:
3ab5aee7 Eric Dumazet 2008-11-16 239 sk_nulls_for_each_rcu(sk, node, &head->chain) {
ce43b03e Eric Dumazet 2012-11-30 240 if (sk->sk_hash != hash)
ce43b03e Eric Dumazet 2012-11-30 241 continue;
ce43b03e Eric Dumazet 2012-11-30 @242 if (likely(INET_MATCH(sk, net, acookie,
ce43b03e Eric Dumazet 2012-11-30 243 saddr, daddr, ports, dif))) {
3ab5aee7 Eric Dumazet 2008-11-16 244 if (unlikely(!atomic_inc_not_zero(&sk->sk_refcnt)))
3ab5aee7 Eric Dumazet 2008-11-16 245 goto begintw;
ce43b03e Eric Dumazet 2012-11-30 246 if (unlikely(!INET_MATCH(sk, net, acookie,
ce43b03e Eric Dumazet 2012-11-30 247 saddr, daddr, ports, dif))) {
3ab5aee7 Eric Dumazet 2008-11-16 248 sock_put(sk);
3ab5aee7 Eric Dumazet 2008-11-16 249 goto begin;
3ab5aee7 Eric Dumazet 2008-11-16 250 }
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* Re: [B.A.T.M.A.N.] [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Sven Eckelmann @ 2012-12-01 18:07 UTC (permalink / raw)
To: b.a.t.m.a.n, bridge, linux-rdma, linux-s390, Andy Gospodarek,
Jay Vosburgh, Divy Le Ray
Cc: Simon Wunderlich, davem, netdev, Simon Wunderlich
In-Reply-To: <1354382991-31350-1-git-send-email-siwu@hrz.tu-chemnitz.de>
[-- Attachment #1: Type: text/plain, Size: 3266 bytes --]
On Saturday 01 December 2012 18:29:51 Simon Wunderlich wrote:
> If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
> may destroy this attempt because it first unlocks rtnl_mutex but may
> lock it again later. The callgraph roughly looks like:
>
> rtnl_unlock()
> netdev_run_todo()
> __rtnl_unlock()
> netdev_wait_allrefs()
> rtnl_lock()
> ...
> __rtnl_unlock()
>
> There are two users which have possible deadlocks and are fixed in this
> patch: batman-adv and bridge. Their problematic access pattern is the same:
>
> notifier_call handler:
> * holds rtnl lock when called
> * calls sysfs code at some point (acquiring sysfs locks)
>
> sysfs code:
> * holds sysfs lock when called
> * calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
> implicitly
>
> Fix this by exporting __rtnl_unlock() to just unlock the mutex without
> implicitly locking again.
>
> Reported-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>
> ---
> Of course, an alternative would be to not lock again after unlocking
> within rtnl_unlock(), or put the todo handling into the locked section.
> I'm not familiar enough with this code to know what would be best.
>
> There are others using rtnl_trylock(), but I'm not sure if they
> are affected.
At least they look like they have a problem in a parallel user scenario
involving another lock and locking order (most of them s_active or a device
lock). So just to list the places and poke some other users. They can better
decide for themself because they know the code.
drivers/infiniband/ulp/ipoib/ipoib_cm.c: if (!rtnl_trylock())
drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock())
drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock())
drivers/net/bonding/bond_alb.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) {
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock())
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c: if (!rtnl_trylock()) /*
synchronize with ifdown */
drivers/s390/net/qeth_l2_main.c: if (rtnl_trylock()) {
drivers/s390/net/qeth_l3_main.c: if (rtnl_trylock()) {
net/bridge/br_sysfs_br.c: if (!rtnl_trylock())
net/bridge/br_sysfs_if.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/core/net-sysfs.c: if (!rtnl_trylock())
net/ipv4/devinet.c: if (!rtnl_trylock()) {
net/ipv6/addrconf.c: if (!rtnl_trylock())
net/ipv6/addrconf.c: if (!rtnl_trylock())
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 17:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114440.331740099591161757.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]
On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
>
> > drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
> > drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is
> negative
>
> It's from adding the completion to the solos skb cb, you can't do
> that. It won't fit on 64-bit when all debugging kconfig options are
> enabled.
This is a sane use of skb refcounting, right? It seems to work, so if
it's OK I'll redo the offending patch with this change to avoid breaking
the bisection, and submit a new pull request.
Very glad I added the BUILD_BUG_ON on the cb struct size now. Perhaps
there should be a generic helper for that? Something like
skb_cb_cast(struct foo_cb, skb) could do it automatically...?
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index e59bcfd..6fe62c5 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -92,7 +92,6 @@ struct pkt_hdr {
};
struct solos_skb_cb {
- struct completion c;
struct atm_vcc *vcc;
uint32_t dma_addr;
};
@@ -853,14 +852,15 @@ static void pclose(struct atm_vcc *vcc)
header->vci = cpu_to_le16(vcc->vci);
header->type = cpu_to_le16(PKT_PCLOSE);
- init_completion(&SKB_CB(skb)->c);
-
+ skb_get(skb);
fpga_queue(card, port, skb, NULL);
- if (!wait_for_completion_timeout(&SKB_CB(skb)->c, 5 * HZ))
+ if (!wait_event_timeout(card->param_wq, !skb_shared(skb), 5 * HZ))
dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n",
port);
+ dev_kfree_skb_any(skb);
+
/* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the
tasklet has finished processing any incoming packets (and, more to
the point, using the vcc pointer). */
@@ -990,10 +990,8 @@ static uint32_t fpga_tx(struct solos_card *card)
atomic_inc(&vcc->stats->tx);
solos_pop(vcc, oldskb);
} else {
- struct pkt_hdr *header = (void *)oldskb->data;
- if (le16_to_cpu(header->type) == PKT_PCLOSE)
- complete(&SKB_CB(oldskb)->c);
dev_kfree_skb_irq(oldskb);
+ wake_up(&card->param_wq);
}
}
}
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply related
* [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Simon Wunderlich @ 2012-12-01 17:29 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Simon Wunderlich
If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock()
may destroy this attempt because it first unlocks rtnl_mutex but may
lock it again later. The callgraph roughly looks like:
rtnl_unlock()
netdev_run_todo()
__rtnl_unlock()
netdev_wait_allrefs()
rtnl_lock()
...
__rtnl_unlock()
There are two users which have possible deadlocks and are fixed in this
patch: batman-adv and bridge. Their problematic access pattern is the same:
notifier_call handler:
* holds rtnl lock when called
* calls sysfs code at some point (acquiring sysfs locks)
sysfs code:
* holds sysfs lock when called
* calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock
implicitly
Fix this by exporting __rtnl_unlock() to just unlock the mutex without
implicitly locking again.
Reported-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
Signed-off-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
---
Of course, an alternative would be to not lock again after unlocking
within rtnl_unlock(), or put the todo handling into the locked section.
I'm not familiar enough with this code to know what would be best.
There are others using rtnl_trylock(), but I'm not sure if they
are affected.
---
net/batman-adv/sysfs.c | 2 +-
net/bridge/br_sysfs_br.c | 2 +-
net/bridge/br_sysfs_if.c | 2 +-
net/core/rtnetlink.c | 1 +
4 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 66518c7..41b74aa 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -635,7 +635,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
ret = batadv_hardif_enable_interface(hard_iface, buff);
unlock:
- rtnl_unlock();
+ __rtnl_unlock();
out:
batadv_hardif_free_ref(hard_iface);
return ret;
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index c5c0593..c122782 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -142,7 +142,7 @@ static ssize_t store_stp_state(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
br_stp_set_enabled(br, val);
- rtnl_unlock();
+ __rtnl_unlock();
return len;
}
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 13b36bd..d99f394 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -223,7 +223,7 @@ static ssize_t brport_store(struct kobject * kobj,
if (ret == 0)
ret = count;
}
- rtnl_unlock();
+ __rtnl_unlock();
}
return ret;
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index fad649a..d95ba6f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -72,6 +72,7 @@ void __rtnl_unlock(void)
{
mutex_unlock(&rtnl_mutex);
}
+EXPORT_SYMBOL(__rtnl_unlock);
void rtnl_unlock(void)
{
--
1.7.10.4
^ permalink raw reply related
* Re: sky2: Correct mistakenly switched read/write sequence
From: David Miller @ 2012-12-01 17:27 UTC (permalink / raw)
To: LinoSanfilippo; +Cc: shemminger, mlindner, netdev
In-Reply-To: <20121201124239.GB3914@neptun>
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Sat, 1 Dec 2012 13:42:39 +0100
> In sky2_all_down() the order of the read()/write() access to B0_IMSK seems to
> be mistakenly switched. The original intention was obviously to avoid PCI write
> posting.
> This patch fixes the order.
>
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
I would say that no such intention exists at all.
The read is there because a long time ago the result as used
to compute an 'imask' variable.
Please see commit:
commit d72ff8fa7f8b344382963721f842256825c4660b
Author: Mike McCormack <mikem@ring3k.org>
Date: Thu May 13 06:12:51 2010 +0000
sky2: Refactor down/up code out of sky2_restart()
Code to bring down all sky2 interfaces and bring it up
again can be reused in sky2_suspend and sky2_resume.
Factor the code to bring the interfaces down into
sky2_all_down and the up code into sky2_all_up.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 17:21 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev, krzysiek
In-Reply-To: <201212011702.qB1H2v07010546@thirdoffive.cmf.nrl.navy.mil>
[-- Attachment #1: Type: text/plain, Size: 984 bytes --]
On Sat, 2012-12-01 at 12:02 -0500, Chas Williams (CONTRACTOR) wrote:
> In message <1354380532.21562.345.camel@shinybook.infradead.org>,David Woodhouse writes:
> >On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
> >> It's from adding the completion to the solos skb cb, you can't do
> >> that. It won't fit on 64-bit when all debugging kconfig options are
> >> enabled.
> >
> >Aha, thanks. Will sort that out. Apologies.
> >
> >The ambassador one may be related to a separate patch which fixes the
> >endless loop in firmware loading? Not in my tree, though.
>
> i think the warning with the ambassador dates back to before the most
> recent patch. probably to the conversion to use the firmware loader.
Possibly not even a bug at all, in fact — GCC is fairly loose with those
warnings. But if it is a bug it's my fault. I'll take a look at that
too. Not tonight though; I'm going out shortly and will only just manage
the solos-pci fix.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: Chas Williams (CONTRACTOR) @ 2012-12-01 17:02 UTC (permalink / raw)
To: David Woodhouse; +Cc: David Miller, netdev, krzysiek
In-Reply-To: <1354380532.21562.345.camel@shinybook.infradead.org>
In message <1354380532.21562.345.camel@shinybook.infradead.org>,David Woodhouse writes:
>On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
>> It's from adding the completion to the solos skb cb, you can't do
>> that. It won't fit on 64-bit when all debugging kconfig options are
>> enabled.
>
>Aha, thanks. Will sort that out. Apologies.
>
>The ambassador one may be related to a separate patch which fixes the
>endless loop in firmware loading? Not in my tree, though.
i think the warning with the ambassador dates back to before the most
recent patch. probably to the conversion to use the firmware loader.
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Woodhouse @ 2012-12-01 16:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114440.331740099591161757.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 404 bytes --]
On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
> It's from adding the completion to the solos skb cb, you can't do
> that. It won't fit on 64-bit when all debugging kconfig options are
> enabled.
Aha, thanks. Will sort that out. Apologies.
The ambassador one may be related to a separate patch which fixes the
endless loop in firmware loading? Not in my tree, though.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Miller @ 2012-12-01 16:44 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, chas, krzysiek
In-Reply-To: <20121201.114320.103305992136431342.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sat, 01 Dec 2012 11:43:20 -0500 (EST)
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Fri, 30 Nov 2012 20:22:09 +0000
>
>> Dave, if you're not now ignoring this thread entirely, please pull into
>> net-next from
>> git://git.infradead.org/users/dwmw2/atm.git
>
> Can you make this actually build first?
>
> drivers/atm/ambassador.c: In function ‘ucode_init’:
> drivers/atm/ambassador.c:1972:19: warning: ‘fw’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
> drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is negative
It's from adding the completion to the solos skb cb, you can't do
that. It won't fit on 64-bit when all debugging kconfig options are
enabled.
^ permalink raw reply
* Re: [PATCH 00/17] ATM fixes for pppoatm/br2684
From: David Miller @ 2012-12-01 16:43 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, chas, krzysiek
In-Reply-To: <1354306929.21562.313.camel@shinybook.infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 30 Nov 2012 20:22:09 +0000
> Dave, if you're not now ignoring this thread entirely, please pull into
> net-next from
> git://git.infradead.org/users/dwmw2/atm.git
Can you make this actually build first?
drivers/atm/ambassador.c: In function ‘ucode_init’:
drivers/atm/ambassador.c:1972:19: warning: ‘fw’ may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is negative
^ permalink raw reply
* Re: [PATCH -next] qlcnic: remove duplicated include from qlcnic_sysfs.c
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: weiyj.lk
Cc: jitendra.kalsaria, sony.chacko, yongjun_wei, linux-driver, netdev
In-Reply-To: <CAPgLHd9rUSTcdop1coRzR00SxwHtetQ5vHwAfLN_Jw1VjaK68g@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Sat, 1 Dec 2012 01:01:25 -0500
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Remove duplicated include.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] myri10ge: fix incorrect use of ntohs()
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: gallatin; +Cc: netdev
In-Reply-To: <1354314686-16149-1-git-send-email-gallatin@myri.com>
From: Andrew Gallatin <gallatin@myri.com>
Date: Fri, 30 Nov 2012 17:31:26 -0500
> 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 incorrectly used
> ntohs() rather than htons() in myri10ge_vlan_rx().
>
> Thanks to Fengguang Wu, Yuanhan Liu's kernel-build tester
> for pointing out this bug.
>
> Signed-off-by: Andrew Gallatin <gallatin@myri.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2] ipv6: unify logic evaluating inet6_dev's accept_ra property
From: David Miller @ 2012-12-01 16:37 UTC (permalink / raw)
To: shmulik.ladkani; +Cc: netdev, yoshfuji, tgraf, tore
In-Reply-To: <1354307159-24193-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Date: Fri, 30 Nov 2012 22:25:59 +0200
> As of 026359b [ipv6: Send ICMPv6 RSes only when RAs are accepted], the
> logic determining whether to send Router Solicitations is identical
> to the logic determining whether kernel accepts Router Advertisements.
>
> However the condition itself is repeated in several code locations.
>
> Unify it by introducing 'ipv6_accept_ra()' accessor.
>
> Also, simplify the condition expression, making it more readable.
> No semantic change.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] tcp: change default tcp hash size
From: David Miller @ 2012-12-01 16:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1354306132.20109.41.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Nov 2012 12:08:52 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> As time passed, available memory increased faster than number of
> concurrent tcp sockets.
>
> As a result, a machine with 4GB of ram gets a hash table
> with 524288 slots, using 8388608 bytes of memory.
>
> Lets change that by a 16x factor (one slot for 128 KB of ram)
>
> Even if a small machine needs a _lot_ of sockets, tcp lookups are now
> very efficient, using one cache line per socket.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Agreed, applied, thanks Eric.
^ 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