Re: Latest PreemptRT patch error on imx35

Re: Latest PreemptRT patch error on imx35

[Uwe Kleine-König]

Hello Tim,

> I just bootet 2.6.31-rc8-rt9 on the phycore-imx35 development board, 
> without any additional patches. The FEC Ethernet Controller went belly 
> up, but the system booted nevertheless. [...]
The problem is that &fep->mii_lock is taken twice.  On a normal (i.e.
non-RT) UP system that doesn't hurt because spinlocks are noops (unless
you have some debugging code activated).

The exact problem is:
	fec_enet_mii (this takes &fep->mii_lock) and calls mip->mii_func
	which happens to be mii_queue.  mii_queue in turn takes
	&fep->mii_lock again.

The correct fix is probably to split mii_queue into a function that
locks and a function that does the actual work.  And then use the latter
in fec_enet_mii.

I'd prepare a patch but my wife just called me to eat some cake :-)

So maybe later today or tomorrow, we'll see.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Latest PreemptRT patch error on imx35

[Tim Sander]

Hi

I just chrosschecked and build a kernel without a preempt RT patch. Two conclusions: 

* A 2.6.31-rc8-rt9 patched kernel is booting under preempt-rt with FEC-Etheret failing as reported or not at all with the configuration: preempt or no-preemption (server). 

* A 2.6.31-rc8 boots on the imx35 board.

Below a output dump of a failed boot of no-preemption 2.6.31-rc8-rt9 kernel

Best regards
Tim


Uncompressing Linux...................................................................................................... done, booting the kernel.
Linux version 2.6.31-rc8-rt9 (sander@dose) (gcc version 4.3.2 (OSELAS.Toolchain-1.99.3) ) #4 Mon Aug 31 13:33:49 CEST 2009
CPU: ARMv6-compatible processor [4117b363] revision 3 (ARMv6TEJ), cr=00c5387f
CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
Machine: Phytec Phycore pcm043
Memory policy: ECC disabled, Data cache writeback
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
Kernel command line: jtag=on console=ttymxc0,115200 video=mx3fb:TX090 ip=172.19.169.71:172.19.169.15::255.255.0.0::: root=/dev/mtdblock3 rootfstype=jffs2 mtdparts=physmap-flash.0:256k(uboot)ro,128k(ubootenv),2048k(kernel),-(root);mxc_nand:
PID hash table entries: 512 (order: 9, 2048 bytes)
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
Memory: 128MB = 128MB total
Memory: 126492KB available (2824K code, 265K data, 96K init, 0K highmem)
Hierarchical RCU implementation.
NR_IRQS:180
MXC GPIO hardware
MXC IRQ initialized
Console: colour dummy device 80x30
Calibrating delay loop... 398.95 BogoMIPS (lpj=1994752)
Mount-cache hash table entries: 512
CPU: Testing write buffer coherency: ok
NET: Registered protocol family 16
L2X0 cache controller enabled
bio: create slab <bio-0> at 0
NET: Registered protocol family 2
IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
TCP established hash table entries: 4096 (order: 3, 32768 bytes)
TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
TCP: Hash tables configured (established 4096 bind 4096)
TCP reno registered
NET: Registered protocol family 1
JFFS2 version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
msgmni has been set to 247
alg: No test for stdrng (krng)
io scheduler noop registered (default)
Serial: IMX driver
imx-uart.0: ttymxc0 at MMIO 0x43f90000 (irq = 45) is a IMX
console [ttymxc0] enabled
imx-uart.1: ttymxc1 at MMIO 0x43f94000 (irq = 32) is a IMX
FEC Ethernet Driver
fec: PHY @ 0x0, ID 0x00221512 -- unknown PHY!
physmap platform flash device: 02000000 at a0000000
physmap-flash.0: Found 1 x16 devices at 0x0 in 16-bit bank
 Intel/Sharp Extended Query Table at 0x010A
 Intel/Sharp Extended Query Table at 0x010A
 Intel/Sharp Extended Query Table at 0x010A
 Intel/Sharp Extended Query Table at 0x010A
 Intel/Sharp Extended Query Table at 0x010A
Using buffer write method
Using auto-unlock on power-up/resume
cfi_cmdset_0001: Erase suspend on write enabled
mtd: partition size too small (0)
4 cmdlinepart partitions found on MTD device physmap-flash.0
Creating 4 MTD partitions on "physmap-flash.0":
0x000000000000-0x000000040000 : "uboot"
0x000000040000-0x000000060000 : "ubootenv"
0x000000060000-0x000000260000 : "kernel"
0x000000260000-0x000002000000 : "root"
mice: PS/2 mouse device common for all mice
Driver for 1-wire Dallas network protocol.
cpuidle: using governor ladder
TCP cubic registered
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 3
drivers/rtc/hctosys.c: unable to open rtc device (rtc0)
IP-Config: Complete:
     device=eth0, addr=172.19.169.71, mask=255.255.0.0, gw=255.255.255.255,
     host=172.19.169.71, domain=, nis-domain=(none),
     bootserver=172.19.169.15, rootserver=172.19.169.15, rootpath=
VFS: Mounted root (jffs2 filesystem) on device 31:3.
Freeing init memory: 96K
Unable to handle kernel paging request at virtual address de609800
pgd = c0004000
[de609800] *pgd=00000000
Internal error: Oops: 805 [#1]
Modules linked in:
CPU: 0    Not tainted  (2.6.31-rc8-rt9 #4)
PC is at __lru_cache_add+0x5c/0x84
LR is at add_to_page_cache_lru+0xa4/0xa8
pc : [<c006ace0>]    lr : [<c0062430>]    psr: a0000013
sp : c7819d04  ip : 00000190  fp : c7407648
r10: c740764c  r9 : 0000005b  r8 : 00100100
r7 : 00200200  r6 : 00000000  r5 : 00000002  r4 : c0305bc8
r3 : c78c0ea9  r2 : c78c0eaa  r1 : c0305d58  r0 : c0342760
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 80004008  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc7818268)
Stack: (0xc7819d04 to 0xc781a000)
9d00:          c0342760 00000000 00000004 0000005b c0342760 00000000 c0069ecc
9d20: c788c380 c7819d38 c74059b8 00000001 c787d9c0 00000004 c0344b78 c033c3f8
9d40: c7407700 00000003 00000000 00000000 00000000 c7407648 c787d9c0 c787d9c0
9d60: 00000000 c0069f4c 00000003 c787d9c0 00000000 c006a328 00000000 00000001
9d80: 00000001 c0062dfc 00000001 c042f580 c0061930 c7819df4 c7819e88 c74075b0
9da0: c787da00 00000001 ffffffff 00000fff 00000000 00000000 00000000 00000000
9dc0: 00000000 c7819ec8 c787d9c0 00000000 c7819df4 c0063bac c031c814 c031c5b0
9de0: 00000000 000000d0 c7819e88 00000001 c031cb60 00000000 00000080 c78821a0
9e00: 00000000 00000080 00000000 00000000 00000000 c7815cc0 c7819e38 c787d9c0
9e20: fffffdee c7819ec8 c7819f18 c0088754 00000000 00000000 00000004 00000000
9e40: 00000000 00000001 ffffffff c787d9c0 00000000 00000000 00000000 00000000
9e60: c7815cc0 00000000 00000000 00000000 00000000 c7815cc0 c0049aac c7819e7c
9e80: c7819e7c c008498c 00000000 00000000 c031c5b0 c79abde0 c78028f0 000f4240
9ea0: 00000080 c7815cc0 c02ff9d0 c7815cc0 00000000 00000000 ffffffff 00000000
9ec0: c7824000 c004e0f8 c78821a0 00000080 00000017 c787d9c0 c78821a0 c7819f18
9ee0: c78821a0 00000080 c7819f78 c02fb8c4 00000000 c00893c8 fffffeff ffffffff
9f00: c7819f18 00000000 000089ed c78821a0 c02fb838 c008dad8 00000000 00000000
9f20: c7819f78 c78821a0 c74075b0 c008dbfc c78821a0 00000080 00000004 c02fb848
9f40: c7818000 c008e36c 00000000 00000000 c7819f60 c02a2740 c02fb8c4 c02fb838
9f60: 00000000 00000000 00000000 00000000 00000000 c0023a70 00000000 00000000
9f80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fa0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fc0: 00000000 c02fb8c4 c031cbd4 00000000 00000000 c0020514 c001bfa8 c001bfa8
9fe0: c001bfa8 c0008464 00000000 00000000 00000000 c002179c fffbfef1 ddcfe9fd
[<c006ace0>] (__lru_cache_add+0x5c/0x84) from [<0000005b>] (0x5b)
Code: e794200c e2823001 e2822002 e3530008 (e7810102)
---[ end trace bb332da858e3d4c4 ]---
Kernel panic - not syncing: Attempted to kill init!
[<c0025618>] (unwind_backtrace+0x0/0xe4) from [<c0246bcc>] (panic+0x3c/0x120)
[<c0246bcc>] (panic+0x3c/0x120) from [<c003913c>] (forget_original_parent+0x240/0x278)
[<c003913c>] (forget_original_parent+0x240/0x278) from [<c0039188>] (exit_notify+0x14/0x154)
[<c0039188>] (exit_notify+0x14/0x154) from [<c00393c4>] (do_exit+0xfc/0x294)
[<c00393c4>] (do_exit+0xfc/0x294) from [<c0024488>] (die+0xac/0xbc)
[<c0024488>] (die+0xac/0xbc) from [<c0026670>] (__do_kernel_fault+0x70/0x80)
[<c0026670>] (__do_kernel_fault+0x70/0x80) from [<c00268b4>] (do_translation_fault+0x6c/0x80)
[<c00268b4>] (do_translation_fault+0x6c/0x80) from [<c0020254>] (do_DataAbort+0x34/0x98)
[<c0020254>] (do_DataAbort+0x34/0x98) from [<c0020a4c>] (__dabt_svc+0x4c/0x60)
Exception stack(0xc7819cb8 to 0xc7819d00)
9ca0:                                                       c0342760 c0305d58
9cc0: c78c0eaa c78c0ea9 c0305bc8 00000002 00000000 00200200 00100100 0000005b
9ce0: c740764c c7407648 00000190 c7819d04 c0062430 c006ace0 a0000013 ffffffff
[<c0020a4c>] (__dabt_svc+0x4c/0x60) from [<c006ace0>] (__lru_cache_add+0x5c/0x84)
[<c006ace0>] (__lru_cache_add+0x5c/0x84) from [<0000005b>] (0x5b)

__lru_cache_add+0x5c/0x84
LR is at add_to_page_cache_lru+0xa4/0xa8
pc : [<c006ace0>]    lr : [<c0062430>]    psr: a0000013
sp : c7819d04  ip : 00000190  fp : c7407648
r10: c740764c  r9 : 0000005b  r8 : 00100100
r7 : 00200200  r6 : 00000000  r5 : 00000002  r4 : c0305bc8
r3 : c78c0ea9  r2 : c78c0eaa  r1 : c0305d58  r0 : c0342760
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 80004008  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc7818268)
Stack: (0xc7819d04 to 0xc781a000)
9d00:          c0342760 00000000 00000004 0000005b c0342760 00000000 c0069ecc
9d20: c788c380 c7819d38 c74059b8 00000001 c787d9c0 00000004 c0344b78 c033c3f8
9d40: c7407700 00000003 00000000 00000000 00000000 c7407648 c787d9c0 c787d9c0
9d60: 00000000 c0069f4c 00000003 c787d9c0 00000000 c006a328 00000000 00000001
9d80: 00000001 c0062dfc 00000001 c042f580 c0061930 c7819df4 c7819e88 c74075b0
9da0: c787da00 00000001 ffffffff 00000fff 00000000 00000000 00000000 00000000
9dc0: 00000000 c7819ec8 c787d9c0 00000000 c7819df4 c0063bac c031c814 c031c5b0
9de0: 00000000 000000d0 c7819e88 00000001 c031cb60 00000000 00000080 c78821a0
9e00: 00000000 00000080 00000000 00000000 00000000 c7815cc0 c7819e38 c787d9c0
9e20: fffffdee c7819ec8 c7819f18 c0088754 00000000 00000000 00000004 00000000
9e40: 00000000 00000001 ffffffff c787d9c0 00000000 00000000 00000000 00000000
9e60: c7815cc0 00000000 00000000 00000000 00000000 c7815cc0 c0049aac c7819e7c
9e80: c7819e7c c008498c 00000000 00000000 c031c5b0 c79abde0 c78028f0 000f4240
9ea0: 00000080 c7815cc0 c02ff9d0 c7815cc0 00000000 00000000 ffffffff 00000000
9ec0: c7824000 c004e0f8 c78821a0 00000080 00000017 c787d9c0 c78821a0 c7819f18
9ee0: c78821a0 00000080 c7819f78 c02fb8c4 00000000 c00893c8 fffffeff ffffffff
9f00: c7819f18 00000000 000089ed c78821a0 c02fb838 c008dad8 00000000 00000000
9f20: c7819f78 c78821a0 c74075b0 c008dbfc c78821a0 00000080 00000004 c02fb848
9f40: c7818000 c008e36c 00000000 00000000 c7819f60 c02a2740 c02fb8c4 c02fb838
9f60: 00000000 00000000 00000000 00000000 00000000 c0023a70 00000000 00000000
9f80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fa0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fc0: 00000000 c02fb8c4 c031cbd4 00000000 00000000 c0020514 c001bfa8 c001bfa8
9fe0: c001bfa8 c0008464 00000000 00000000 00000000 c002179c fffbfef1 ddcfe9fd
[<c006ace0>] (__lru_cache_add+0x5c/0x84) from [<0000005b>] (0x5b)
Code: e794200c e2823001 e2822002 e3530008 (e7810102)
---[ end trace bb332da858e3d4c4 ]---
Kernel panic - not syncing: Attempted to kill init!
[<c0025618>] (unwind_backtrace+0x0/0xe4) from [<c0246bcc>] (panic+0x3c/0x120)
[<c0246bcc>] (panic+0x3c/0x120) from [<c003913c>] (forget_original_parent+0x240/0x278)
[<c003913c>] (forget_original_parent+0x240/0x278) from [<c0039188>] (exit_notify+0x14/0x154)
[<c0039188>] (exit_notify+0x14/0x154) from [<c00393c4>] (do_exit+0xfc/0x294)
[<c00393c4>] (do_exit+0xfc/0x294) from [<c0024488>] (die+0xac/0xbc)
[<c0024488>] (die+0xac/0xbc) from [<c0026670>] (__do_kernel_fault+0x70/0x80)
[<c0026670>] (__do_kernel_fault+0x70/0x80) from [<c00268b4>] (do_translation_fault+0x6c/0x80)
[<c00268b4>] (do_translation_fault+0x6c/0x80) from [<c0020254>] (do_DataAbort+0x34/0x98)
[<c0020254>] (do_DataAbort+0x34/0x98) from [<c0020a4c>] (__dabt_svc+0x4c/0x60)
Exception stack(0xc7819cb8 to 0xc7819d00)
9ca0:                                                       c0342760 c0305d58
9cc0: c78c0eaa c78c0ea9 c0305bc8 00000002 00000000 00200200 00100100 0000005b
9ce0: c740764c c7407648 00000190 c7819d04 c0062430 c006ace0 a0000013 ffffffff
[<c0020a4c>] (__dabt_svc+0x4c/0x60) from [<c006ace0>] (__lru_cache_add+0x5c/0x84)
[<c006ace0>] (__lru_cache_add+0x5c/0x84) from [<0000005b>] (0x5b)
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Re: Latest PreemptRT patch error on imx35

[Tim Sander]

Hi Uwe
> The exact problem is:
>       fec_enet_mii (this takes &fep->mii_lock) and calls mip->mii_func
>       which happens to be mii_queue.  mii_queue in turn takes
>       &fep->mii_lock again.
Mh, lets see, i tried this:
--- linux-2.6.30/drivers/net/fec.orig.c 2009-08-31 18:01:25.000000000 +0200
+++ linux-2.6.30/drivers/net/fec.c      2009-08-31 18:06:31.000000000 +0200
@@ -622,8 +622,7 @@
                goto unlock;
        }

-       if (mip->mii_func != NULL)
-               (*(mip->mii_func))(readl(fep->hwp + FEC_MII_DATA), dev);
+       mii_queue_locked(readl(fep->hwp + FEC_MII_DATA), dev);

        mii_head = mip->mii_next;
        mip->mii_next = mii_free;
@@ -637,17 +636,9 @@
 }

 static int
-mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct 
net_device *))
+mii_queue_locked(struct net_device *dev, int regval, void (*func)(uint, 
struct net_device *))
 {
-       struct fec_enet_private *fep;
-       unsigned long   flags;
-       mii_list_t      *mip;
        int             retval;
-
-       /* Add PHY address to register command */
-       fep = netdev_priv(dev);
-       spin_lock_irqsave(&fep->mii_lock, flags);
-
        regval |= fep->phy_addr << 23;
        retval = 0;

@@ -666,6 +657,22 @@
        } else {
                retval = 1;
        }
+       return retval;
+}
+
+static int
+mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct 
net_device *))
+{
+       struct fec_enet_private *fep;
+       unsigned long   flags;
+       mii_list_t      *mip;
+       int             retval;
+
+       /* Add PHY address to register command */
+       fep = netdev_priv(dev);
+       spin_lock_irqsave(&fep->mii_lock, flags);
+
+       retval=mii_queue_locked(dev,regval,net_device);

        spin_unlock_irqrestore(&fep->mii_lock, flags);
        return retval;

---
But it still gives me:
FEC Ethernet Driver
kernel BUG at kernel/rtmutex.c:807!
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 805 [#1] PREEMPT
Modules linked in:
CPU: 0    Not tainted  (2.6.31-rc8-rt9 #1)
PC is at __bug+0x1c/0x28
LR is at __bug+0x18/0x28
pc : [<c0024184>]    lr : [<c0024180>]    psr: 00000193
sp : c7891ef0  ip : c7891e30  fp : 00000002
r10: c7911c24  r9 : c031f9cc  r8 : c0351e54
r7 : 40000113  r6 : c0195c74  r5 : c7890000  r4 : 600e0000
r3 : 00000000  r2 : 00000001  r1 : c7890000  r0 : 0000002a
Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 80004008  DAC: 00000017
Process irq/57-fec (pid: 243, stack limit = 0xc7890268)
Stack: (0xc7891ef0 to 0xc7892000)
1ee0:                                     c025fb3c c025fd38 00000000 40000113
1f00: ffffffff c7891f10 c025fcd8 c0032124 00000000 c7891f20 c0031764 c781e01c
1f20: c78ffb40 00000017 00000000 c7819b20 c7891f74 600e0000 c7911800 c0195c74
1f40: c7911c24 c0351e54 c031f9cc c0351e58 00000002 c0195dfc c0351d64 c7911b20
1f60: c7911800 c7911c24 c0351e54 c0196794 c025ee80 c031f9c4 c79885a0 c7890000
1f80: c031f988 c79885c8 c031f9e4 c0062950 00000000 00000032 c7891fbc c781fe4c
1fa0: c79885a0 c006281c 00000000 00000000 00000000 00000000 00000000 c004c5e8
1fc0: 00000000 00000000 c7891fc8 c7891fc8 c7891fd0 c7891fd0 00000000 00000000
1fe0: c7891fe0 c7891fe0 00000000 00000000 00000000 c00218b4 ffebfef3 ddddebfd
[<c0024184>] (__bug+0x1c/0x28) from [<c025fd38>] 
(rt_spin_lock_slowlock+0x220/0x234)
[<c025fd38>] (rt_spin_lock_slowlock+0x220/0x234) from [<c0195dfc>] 
(mii_queue+0x20/0xb4)
[<c0195dfc>] (mii_queue+0x20/0xb4) from [<c0196794>] 
(fec_enet_interrupt+0x84/0x104)
[<c0196794>] (fec_enet_interrupt+0x84/0x104) from [<c0062950>] 
(irq_thread+0x134/0x1c0)
[<c0062950>] (irq_thread+0x134/0x1c0) from [<c004c5e8>] (kthread+0x7c/0x84)
[<c004c5e8>] (kthread+0x7c/0x84) from [<c00218b4>] 
(kernel_thread_exit+0x0/0x8)
Code: e1a01000 e59f000c eb08e8b0 e3a03000 (e5833000)
---[ end trace 25de9c8d632021f6 ]---
exiting task "irq/57-fec" (243) is an active IRQ thread (irq 57)

Best regards
Tim

[PATCH 1/2] fec: fix recursive locking of mii_lock

[Uwe Kleine-König]

mii_discover_phy is only called by fec_enet_mii (via mip->mii_func).  So
&fep->mii_lock is already held and mii_discover_phy must not call
mii_queue which locks &fep->mii_lock, too.

This was noticed by lockdep:

	=============================================
	[ INFO: possible recursive locking detected ]
	2.6.31-rc8-00038-g37d0892 #109
	---------------------------------------------
	swapper/1 is trying to acquire lock:
	 (&fep->mii_lock){-.....}, at: [<c01569f8>] mii_queue+0x2c/0xcc

	but task is already holding lock:
	 (&fep->mii_lock){-.....}, at: [<c0156328>] fec_enet_interrupt+0x78/0x460

	other info that might help us debug this:
	2 locks held by swapper/1:
	 #0:  (rtnl_mutex){+.+.+.}, at: [<c0183534>] rtnl_lock+0x18/0x20
	 #1:  (&fep->mii_lock){-.....}, at: [<c0156328>] fec_enet_interrupt+0x78/0x460

	stack backtrace:
	Backtrace:
	[<c00226fc>] (dump_backtrace+0x0/0x108) from [<c01eac14>] (dump_stack+0x18/0x1c)
	 r6:c781d118 r5:c03e41d8 r4:00000001
	[<c01eabfc>] (dump_stack+0x0/0x1c) from [<c005bae4>] (__lock_acquire+0x1a20/0x1a88)
	[<c005a0c4>] (__lock_acquire+0x0/0x1a88) from [<c005bbac>] (lock_acquire+0x60/0x74)
	[<c005bb4c>] (lock_acquire+0x0/0x74) from [<c01edda8>] (_spin_lock_irqsave+0x54/0x68)
	 r7:60000093 r6:c01569f8 r5:c785e468 r4:00000000
	[<c01edd54>] (_spin_lock_irqsave+0x0/0x68) from [<c01569f8>] (mii_queue+0x2c/0xcc)
	 r7:c785e468 r6:c0156b24 r5:600a0000 r4:c785e000
	[<c01569cc>] (mii_queue+0x0/0xcc) from [<c0156b78>] (mii_discover_phy+0x54/0xa8)
	 r8:00000002 r7:00000032 r6:c785e000 r5:c785e360 r4:c785e000
	[<c0156b24>] (mii_discover_phy+0x0/0xa8) from [<c0156354>] (fec_enet_interrupt+0xa4/0x460)
	 r5:c785e360 r4:c077a170
	[<c01562b0>] (fec_enet_interrupt+0x0/0x460) from [<c0066674>] (handle_IRQ_event+0x48/0x120)
	[<c006662c>] (handle_IRQ_event+0x0/0x120) from [<c0068438>] (handle_level_irq+0x94/0x11c)
	...

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Matt Waddel <Matt.Waddel@freescale.com>
Cc: netdev@vger.kernel.org
Cc: Tim Sander <tim01@vlsi.informatik.tu-darmstadt.de>
---
 drivers/net/fec.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index c9fd82d..ef82606 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -637,16 +637,15 @@ unlock:
 }
 
 static int
-mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct net_device *))
+mii_queue_unlocked(struct net_device *dev, int regval,
+		void (*func)(uint, struct net_device *))
 {
 	struct fec_enet_private *fep;
-	unsigned long	flags;
 	mii_list_t	*mip;
 	int		retval;
 
 	/* Add PHY address to register command */
 	fep = netdev_priv(dev);
-	spin_lock_irqsave(&fep->mii_lock, flags);
 
 	regval |= fep->phy_addr << 23;
 	retval = 0;
@@ -667,6 +666,19 @@ mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct net_devi
 		retval = 1;
 	}
 
+	return retval;
+}
+
+static int
+mii_queue(struct net_device *dev, int regval,
+		void (*func)(uint, struct net_device *))
+{
+	struct fec_enet_private *fep;
+	unsigned long   flags;
+	int             retval;
+	fep = netdev_priv(dev);
+	spin_lock_irqsave(&fep->mii_lock, flags);
+	retval = mii_queue_unlocked(dev, regval, func);
 	spin_unlock_irqrestore(&fep->mii_lock, flags);
 	return retval;
 }
@@ -1373,11 +1385,11 @@ mii_discover_phy(uint mii_reg, struct net_device *dev)
 
 			/* Got first part of ID, now get remainder */
 			fep->phy_id = phytype << 16;
-			mii_queue(dev, mk_mii_read(MII_REG_PHYIR2),
+			mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR2),
 							mii_discover_phy3);
 		} else {
 			fep->phy_addr++;
-			mii_queue(dev, mk_mii_read(MII_REG_PHYIR1),
+			mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR1),
 							mii_discover_phy);
 		}
 	} else {
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] fec: don't enable irqs in hard irq context

[Uwe Kleine-König]

fec_enet_mii, fec_enet_rx and fec_enet_tx are both only called by
fec_enet_interrupt in interrupt context.  So they must not use
spin_lock_irq/spin_unlock_irq.

This fixes:
	WARNING: at kernel/lockdep.c:2140 trace_hardirqs_on_caller+0x130/0x194()
	...

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Matt Waddel <Matt.Waddel@freescale.com>
Cc: netdev@vger.kernel.org
Cc: Tim Sander <tim01@vlsi.informatik.tu-darmstadt.de>
---
 drivers/net/fec.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index ef82606..9c49d56 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -427,7 +427,7 @@ fec_enet_tx(struct net_device *dev)
 	struct	sk_buff	*skb;
 
 	fep = netdev_priv(dev);
-	spin_lock_irq(&fep->hw_lock);
+	spin_lock(&fep->hw_lock);
 	bdp = fep->dirty_tx;
 
 	while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
@@ -486,7 +486,7 @@ fec_enet_tx(struct net_device *dev)
 		}
 	}
 	fep->dirty_tx = bdp;
-	spin_unlock_irq(&fep->hw_lock);
+	spin_unlock(&fep->hw_lock);
 }
 
 
@@ -509,7 +509,7 @@ fec_enet_rx(struct net_device *dev)
 	flush_cache_all();
 #endif
 
-	spin_lock_irq(&fep->hw_lock);
+	spin_lock(&fep->hw_lock);
 
 	/* First, grab all of the stats for the incoming packet.
 	 * These get messed up if we get called due to a busy condition.
@@ -604,7 +604,7 @@ rx_processing_done:
 	}
 	fep->cur_rx = bdp;
 
-	spin_unlock_irq(&fep->hw_lock);
+	spin_unlock(&fep->hw_lock);
 }
 
 /* called from interrupt context */
@@ -615,7 +615,7 @@ fec_enet_mii(struct net_device *dev)
 	mii_list_t	*mip;
 
 	fep = netdev_priv(dev);
-	spin_lock_irq(&fep->mii_lock);
+	spin_lock(&fep->mii_lock);
 
 	if ((mip = mii_head) == NULL) {
 		printk("MII and no head!\n");
@@ -633,7 +633,7 @@ fec_enet_mii(struct net_device *dev)
 		writel(mip->mii_regval, fep->hwp + FEC_MII_DATA);
 
 unlock:
-	spin_unlock_irq(&fep->mii_lock);
+	spin_unlock(&fep->mii_lock);
 }
 
 static int
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] fec: don't enable irqs in hard irq context

[Greg Ungerer]

Hi Uwe,

Uwe Kleine-König wrote:
> fec_enet_mii, fec_enet_rx and fec_enet_tx are both only called by
> fec_enet_interrupt in interrupt context.  So they must not use
> spin_lock_irq/spin_unlock_irq.
> 
> This fixes:
> 	WARNING: at kernel/lockdep.c:2140 trace_hardirqs_on_caller+0x130/0x194()
> 	...
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Greg Ungerer <gerg@uclinux.org>

Looks good.

Acked-by: Greg Ungerer <gerg@uclinux.org>


> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Matt Waddel <Matt.Waddel@freescale.com>
> Cc: netdev@vger.kernel.org
> Cc: Tim Sander <tim01@vlsi.informatik.tu-darmstadt.de>
> ---
>  drivers/net/fec.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index ef82606..9c49d56 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -427,7 +427,7 @@ fec_enet_tx(struct net_device *dev)
>  	struct	sk_buff	*skb;
>  
>  	fep = netdev_priv(dev);
> -	spin_lock_irq(&fep->hw_lock);
> +	spin_lock(&fep->hw_lock);
>  	bdp = fep->dirty_tx;
>  
>  	while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
> @@ -486,7 +486,7 @@ fec_enet_tx(struct net_device *dev)
>  		}
>  	}
>  	fep->dirty_tx = bdp;
> -	spin_unlock_irq(&fep->hw_lock);
> +	spin_unlock(&fep->hw_lock);
>  }
>  
>  
> @@ -509,7 +509,7 @@ fec_enet_rx(struct net_device *dev)
>  	flush_cache_all();
>  #endif
>  
> -	spin_lock_irq(&fep->hw_lock);
> +	spin_lock(&fep->hw_lock);
>  
>  	/* First, grab all of the stats for the incoming packet.
>  	 * These get messed up if we get called due to a busy condition.
> @@ -604,7 +604,7 @@ rx_processing_done:
>  	}
>  	fep->cur_rx = bdp;
>  
> -	spin_unlock_irq(&fep->hw_lock);
> +	spin_unlock(&fep->hw_lock);
>  }
>  
>  /* called from interrupt context */
> @@ -615,7 +615,7 @@ fec_enet_mii(struct net_device *dev)
>  	mii_list_t	*mip;
>  
>  	fep = netdev_priv(dev);
> -	spin_lock_irq(&fep->mii_lock);
> +	spin_lock(&fep->mii_lock);
>  
>  	if ((mip = mii_head) == NULL) {
>  		printk("MII and no head!\n");
> @@ -633,7 +633,7 @@ fec_enet_mii(struct net_device *dev)
>  		writel(mip->mii_regval, fep->hwp + FEC_MII_DATA);
>  
>  unlock:
> -	spin_unlock_irq(&fep->mii_lock);
> +	spin_unlock(&fep->mii_lock);
>  }
>  
>  static int

-- 
-----------------------------------------------------------------------

Re: [PATCH 1/2] fec: fix recursive locking of mii_lock

[Greg Ungerer]

Hi Uwe,

Uwe Kleine-König wrote:
> mii_discover_phy is only called by fec_enet_mii (via mip->mii_func).  So
> &fep->mii_lock is already held and mii_discover_phy must not call
> mii_queue which locks &fep->mii_lock, too.
> 
> This was noticed by lockdep:
> 
> 	=============================================
> 	[ INFO: possible recursive locking detected ]
> 	2.6.31-rc8-00038-g37d0892 #109
> 	---------------------------------------------
> 	swapper/1 is trying to acquire lock:
> 	 (&fep->mii_lock){-.....}, at: [<c01569f8>] mii_queue+0x2c/0xcc
> 
> 	but task is already holding lock:
> 	 (&fep->mii_lock){-.....}, at: [<c0156328>] fec_enet_interrupt+0x78/0x460
> 
> 	other info that might help us debug this:
> 	2 locks held by swapper/1:
> 	 #0:  (rtnl_mutex){+.+.+.}, at: [<c0183534>] rtnl_lock+0x18/0x20
> 	 #1:  (&fep->mii_lock){-.....}, at: [<c0156328>] fec_enet_interrupt+0x78/0x460
> 
> 	stack backtrace:
> 	Backtrace:
> 	[<c00226fc>] (dump_backtrace+0x0/0x108) from [<c01eac14>] (dump_stack+0x18/0x1c)
> 	 r6:c781d118 r5:c03e41d8 r4:00000001
> 	[<c01eabfc>] (dump_stack+0x0/0x1c) from [<c005bae4>] (__lock_acquire+0x1a20/0x1a88)
> 	[<c005a0c4>] (__lock_acquire+0x0/0x1a88) from [<c005bbac>] (lock_acquire+0x60/0x74)
> 	[<c005bb4c>] (lock_acquire+0x0/0x74) from [<c01edda8>] (_spin_lock_irqsave+0x54/0x68)
> 	 r7:60000093 r6:c01569f8 r5:c785e468 r4:00000000
> 	[<c01edd54>] (_spin_lock_irqsave+0x0/0x68) from [<c01569f8>] (mii_queue+0x2c/0xcc)
> 	 r7:c785e468 r6:c0156b24 r5:600a0000 r4:c785e000
> 	[<c01569cc>] (mii_queue+0x0/0xcc) from [<c0156b78>] (mii_discover_phy+0x54/0xa8)
> 	 r8:00000002 r7:00000032 r6:c785e000 r5:c785e360 r4:c785e000
> 	[<c0156b24>] (mii_discover_phy+0x0/0xa8) from [<c0156354>] (fec_enet_interrupt+0xa4/0x460)
> 	 r5:c785e360 r4:c077a170
> 	[<c01562b0>] (fec_enet_interrupt+0x0/0x460) from [<c0066674>] (handle_IRQ_event+0x48/0x120)
> 	[<c006662c>] (handle_IRQ_event+0x0/0x120) from [<c0068438>] (handle_level_irq+0x94/0x11c)
> 	...
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Greg Ungerer <gerg@uclinux.org>

Looks good too.

Acked-by: Greg Ungerer <gerg@uclinux.org>


> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Matt Waddel <Matt.Waddel@freescale.com>
> Cc: netdev@vger.kernel.org
> Cc: Tim Sander <tim01@vlsi.informatik.tu-darmstadt.de>
> ---
>  drivers/net/fec.c |   22 +++++++++++++++++-----
>  1 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index c9fd82d..ef82606 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -637,16 +637,15 @@ unlock:
>  }
>  
>  static int
> -mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct net_device *))
> +mii_queue_unlocked(struct net_device *dev, int regval,
> +		void (*func)(uint, struct net_device *))
>  {
>  	struct fec_enet_private *fep;
> -	unsigned long	flags;
>  	mii_list_t	*mip;
>  	int		retval;
>  
>  	/* Add PHY address to register command */
>  	fep = netdev_priv(dev);
> -	spin_lock_irqsave(&fep->mii_lock, flags);
>  
>  	regval |= fep->phy_addr << 23;
>  	retval = 0;
> @@ -667,6 +666,19 @@ mii_queue(struct net_device *dev, int regval, void (*func)(uint, struct net_devi
>  		retval = 1;
>  	}
>  
> +	return retval;
> +}
> +
> +static int
> +mii_queue(struct net_device *dev, int regval,
> +		void (*func)(uint, struct net_device *))
> +{
> +	struct fec_enet_private *fep;
> +	unsigned long   flags;
> +	int             retval;
> +	fep = netdev_priv(dev);
> +	spin_lock_irqsave(&fep->mii_lock, flags);
> +	retval = mii_queue_unlocked(dev, regval, func);
>  	spin_unlock_irqrestore(&fep->mii_lock, flags);
>  	return retval;
>  }
> @@ -1373,11 +1385,11 @@ mii_discover_phy(uint mii_reg, struct net_device *dev)
>  
>  			/* Got first part of ID, now get remainder */
>  			fep->phy_id = phytype << 16;
> -			mii_queue(dev, mk_mii_read(MII_REG_PHYIR2),
> +			mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR2),
>  							mii_discover_phy3);
>  		} else {
>  			fep->phy_addr++;
> -			mii_queue(dev, mk_mii_read(MII_REG_PHYIR1),
> +			mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR1),
>  							mii_discover_phy);
>  		}
>  	} else {

-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com

Re: [PATCH 1/2] fec: fix recursive locking of mii_lock

[David Miller]

From: Greg Ungerer <gerg@snapgear.com>
Date: Thu, 03 Sep 2009 11:16:16 +1000

> Hi Uwe,
> 
> Uwe Kleine-König wrote:
>> mii_discover_phy is only called by fec_enet_mii (via mip->mii_func).
>> So
>> &fep->mii_lock is already held and mii_discover_phy must not call
>> mii_queue which locks &fep->mii_lock, too.
>> This was noticed by lockdep:
 ...
>> 	...
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> Cc: Greg Ungerer <gerg@uclinux.org>
> 
> Looks good too.
> 
> Acked-by: Greg Ungerer <gerg@uclinux.org>

Applied to net-next-2.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] fec: don't enable irqs in hard irq context

[David Miller]

From: Greg Ungerer <gerg@snapgear.com>
Date: Thu, 03 Sep 2009 11:14:24 +1000

> Hi Uwe,
> 
> Uwe Kleine-König wrote:
>> fec_enet_mii, fec_enet_rx and fec_enet_tx are both only called by
>> fec_enet_interrupt in interrupt context.  So they must not use
>> spin_lock_irq/spin_unlock_irq.
>> This fixes:
>> 	WARNING: at kernel/lockdep.c:2140
>> 	trace_hardirqs_on_caller+0x130/0x194()
>> 	...
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> Cc: Greg Ungerer <gerg@uclinux.org>
> 
> Looks good.
> 
> Acked-by: Greg Ungerer <gerg@uclinux.org>

Applied to net-next-2.6