Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
> Just tested this patch on a macbook pro with i915 and radeon. First I get a > nasty lockdep splat, then it locks up completely: > > [ 13.507373] fb: conflicting fb hw usage radeondrmfb vs EFI VGA - removing > generic driver The EFI driver is known to need other work of its own. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Hey,
Op 13-01-13 01:02, Borislav Petkov schreef:
> On Sat, Jan 12, 2013 at 01:13:23PM -0800, Andrew Morton wrote:
>> Florian has been taking a month or two's downtime (now expired, I
>> think) so I've been waiting for him to reappear to process this one for
>> 3.8.
>>
>> Meanwhile, I guess we could put it into mainline ;) It has been in
>> -next for a month.
>>
>> From: Alan Cox
>> Subject: fb: rework locking to fix lock ordering on takeover
>>
>> Adjust the console layer to allow a take over call where the caller
>> already holds the locks. Make the fb layer lock in order.
>>
>> This is partly a band aid, the fb layer is terminally confused about the
>> locking rules it uses for its notifiers it seems.
>>
>> [a...@linux-foundation.org: remove stray non-ascii char, tidy comment]
>> [a...@linux-foundation.org: export do_take_over_console()]
>> Signed-off-by: Alan Cox
>> Cc: Florian Tobias Schandinat
>> Cc: Stephen Rothwell
>> Signed-off-by: Andrew Morton
> Yes, that's the one, modulo Andrew's fixes, which I've been running.
> Irregardless, I'll run this one tomorrow just in case, because it
> triggers here so easily.
>
Just tested this patch on a macbook pro with i915 and radeon. First I get a
nasty lockdep splat, then it locks up completely:
[ 13.507373] fb: conflicting fb hw usage radeondrmfb vs EFI VGA - removing
generic driver
[ 13.507412]
[ 13.507413] ==
[ 13.507413] [ INFO: possible circular locking dependency detected ]
[ 13.507414] 3.8.0-rc3-patser+ #910 Not tainted
[ 13.507415] ---
[ 13.507415] modprobe/554 is trying to acquire lock:
[ 13.507421] (console_lock){+.+.+.}, at: []
unbind_con_driver+0x37/0x210
[ 13.507422]
[ 13.507422] but task is already holding lock:
[ 13.507425] ((fb_notifier_list).rwsem){.+.+.+}, at: []
__blocking_notifier_call_chain+0x3d/0x80
[ 13.507426]
[ 13.507426] which lock already depends on the new lock.
[ 13.507426]
[ 13.507426]
[ 13.507426] the existing dependency chain (in reverse order) is:
[ 13.507428]
[ 13.507428] -> #1 ((fb_notifier_list).rwsem){.+.+.+}:
[ 13.507431][] lock_acquire+0x96/0xc0
[ 13.507434][] down_read+0x42/0x57
[ 13.507435][]
__blocking_notifier_call_chain+0x3d/0x80
[ 13.507437][]
blocking_notifier_call_chain+0x11/0x20
[ 13.507439][] fb_notifier_call_chain+0x16/0x20
[ 13.507441][] register_framebuffer+0x1c3/0x2e0
[ 13.507444][] efifb_probe+0x402/0x489
[ 13.507446][] platform_drv_probe+0x3e/0x70
[ 13.507448][] driver_probe_device+0x76/0x240
[ 13.507450][] __driver_attach+0xa3/0xb0
[ 13.507451][] bus_for_each_dev+0x56/0x90
[ 13.507452][] driver_attach+0x19/0x20
[ 13.507454][] bus_add_driver+0x188/0x270
[ 13.507455][] driver_register+0x75/0x150
[ 13.507457][] platform_driver_register+0x41/0x50
[ 13.507458][] platform_driver_probe+0x16/0xa0
[ 13.507460][] efifb_init+0x277/0x292
[ 13.507462][] do_one_initcall+0x3a/0x170
[ 13.507464][] kernel_init+0x11d/0x290
[ 13.507466][] ret_from_fork+0x7c/0xb0
[ 13.507467]
[ 13.507467] -> #0 (console_lock){+.+.+.}:
[ 13.507469][] __lock_acquire+0x168f/0x1d90
[ 13.507471][] lock_acquire+0x96/0xc0
[ 13.507474][] console_lock+0x6f/0x80
[ 13.507475][] unbind_con_driver+0x37/0x210
[ 13.507477][] fbcon_event_notify+0x477/0x920
[ 13.507478][] notifier_call_chain+0x58/0xb0
[ 13.507479][]
__blocking_notifier_call_chain+0x53/0x80
[ 13.507481][]
blocking_notifier_call_chain+0x11/0x20
[ 13.507482][] fb_notifier_call_chain+0x16/0x20
[ 13.507484][] do_unregister_framebuffer+0x62/0x100
[ 13.507485][]
do_remove_conflicting_framebuffers+0x154/0x180
[ 13.507487][]
remove_conflicting_framebuffers+0x3a/0x60
[ 13.507501][] radeon_pci_probe+0x84/0xc0 [radeon]
[ 13.507503][] local_pci_probe+0x46/0x80
[ 13.507505][] pci_device_probe+0xf9/0x120
[ 13.507506][] driver_probe_device+0x76/0x240
[ 13.507507][] __driver_attach+0xa3/0xb0
[ 13.507508][] bus_for_each_dev+0x56/0x90
[ 13.507510][] driver_attach+0x19/0x20
[ 13.507511][] bus_add_driver+0x188/0x270
[ 13.507512][] driver_register+0x75/0x150
[ 13.507514][] __pci_register_driver+0x5f/0x70
[ 13.507520][] drm_pci_init+0x11a/0x130 [drm]
[ 13.507528][] radeon_init+0xec/0x1000 [radeon]
[ 13.507530][] do_one_initcall+0x3a/0x170
[ 13.507532][] load_module+0x1a52/0x2020
[ 13.507533][] sys_init_module+0xd1/0x100
[ 13.507535][] system_call_fastpath+0x16/0x1b
[ 13.507535]
[ 13.507535] other info that might help us debug this:
[ 13.507535]
[ 13.507535]
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Hey,
Op 13-01-13 01:02, Borislav Petkov schreef:
On Sat, Jan 12, 2013 at 01:13:23PM -0800, Andrew Morton wrote:
Florian has been taking a month or two's downtime (now expired, I
think) so I've been waiting for him to reappear to process this one for
3.8.
Meanwhile, I guess we could put it into mainline ;) It has been in
-next for a month.
From: Alan Cox a...@linux.intel.com
Subject: fb: rework locking to fix lock ordering on takeover
Adjust the console layer to allow a take over call where the caller
already holds the locks. Make the fb layer lock in order.
This is partly a band aid, the fb layer is terminally confused about the
locking rules it uses for its notifiers it seems.
[a...@linux-foundation.org: remove stray non-ascii char, tidy comment]
[a...@linux-foundation.org: export do_take_over_console()]
Signed-off-by: Alan Cox a...@linux.intel.com
Cc: Florian Tobias Schandinat florianschandi...@gmx.de
Cc: Stephen Rothwell s...@canb.auug.org.au
Signed-off-by: Andrew Morton a...@linux-foundation.org
Yes, that's the one, modulo Andrew's fixes, which I've been running.
Irregardless, I'll run this one tomorrow just in case, because it
triggers here so easily.
Just tested this patch on a macbook pro with i915 and radeon. First I get a
nasty lockdep splat, then it locks up completely:
[ 13.507373] fb: conflicting fb hw usage radeondrmfb vs EFI VGA - removing
generic driver
[ 13.507412]
[ 13.507413] ==
[ 13.507413] [ INFO: possible circular locking dependency detected ]
[ 13.507414] 3.8.0-rc3-patser+ #910 Not tainted
[ 13.507415] ---
[ 13.507415] modprobe/554 is trying to acquire lock:
[ 13.507421] (console_lock){+.+.+.}, at: [813f6ca7]
unbind_con_driver+0x37/0x210
[ 13.507422]
[ 13.507422] but task is already holding lock:
[ 13.507425] ((fb_notifier_list).rwsem){.+.+.+}, at: [8106d96d]
__blocking_notifier_call_chain+0x3d/0x80
[ 13.507426]
[ 13.507426] which lock already depends on the new lock.
[ 13.507426]
[ 13.507426]
[ 13.507426] the existing dependency chain (in reverse order) is:
[ 13.507428]
[ 13.507428] - #1 ((fb_notifier_list).rwsem){.+.+.+}:
[ 13.507431][8109e946] lock_acquire+0x96/0xc0
[ 13.507434][816f8442] down_read+0x42/0x57
[ 13.507435][8106d96d]
__blocking_notifier_call_chain+0x3d/0x80
[ 13.507437][8106d9c1]
blocking_notifier_call_chain+0x11/0x20
[ 13.507439][8138e816] fb_notifier_call_chain+0x16/0x20
[ 13.507441][8138fd23] register_framebuffer+0x1c3/0x2e0
[ 13.507444][81cdfabf] efifb_probe+0x402/0x489
[ 13.507446][81412e0e] platform_drv_probe+0x3e/0x70
[ 13.507448][81410e96] driver_probe_device+0x76/0x240
[ 13.507450][81411103] __driver_attach+0xa3/0xb0
[ 13.507451][8140f2f6] bus_for_each_dev+0x56/0x90
[ 13.507452][814109f9] driver_attach+0x19/0x20
[ 13.507454][81410598] bus_add_driver+0x188/0x270
[ 13.507455][81411635] driver_register+0x75/0x150
[ 13.507457][814126a1] platform_driver_register+0x41/0x50
[ 13.507458][814126c6] platform_driver_probe+0x16/0xa0
[ 13.507460][81cdfdbd] efifb_init+0x277/0x292
[ 13.507462][810001fa] do_one_initcall+0x3a/0x170
[ 13.507464][816df85d] kernel_init+0x11d/0x290
[ 13.507466][817008ac] ret_from_fork+0x7c/0xb0
[ 13.507467]
[ 13.507467] - #0 (console_lock){+.+.+.}:
[ 13.507469][8109dccf] __lock_acquire+0x168f/0x1d90
[ 13.507471][8109e946] lock_acquire+0x96/0xc0
[ 13.507474][81048dff] console_lock+0x6f/0x80
[ 13.507475][813f6ca7] unbind_con_driver+0x37/0x210
[ 13.507477][8139dbe7] fbcon_event_notify+0x477/0x920
[ 13.507478][816fe318] notifier_call_chain+0x58/0xb0
[ 13.507479][8106d983]
__blocking_notifier_call_chain+0x53/0x80
[ 13.507481][8106d9c1]
blocking_notifier_call_chain+0x11/0x20
[ 13.507482][8138e816] fb_notifier_call_chain+0x16/0x20
[ 13.507484][8138f902] do_unregister_framebuffer+0x62/0x100
[ 13.507485][8138fb34]
do_remove_conflicting_framebuffers+0x154/0x180
[ 13.507487][8138fe7a]
remove_conflicting_framebuffers+0x3a/0x60
[ 13.507501][a02111a4] radeon_pci_probe+0x84/0xc0 [radeon]
[ 13.507503][813745c6] local_pci_probe+0x46/0x80
[ 13.507505][81375e19] pci_device_probe+0xf9/0x120
[ 13.507506][81410e96] driver_probe_device+0x76/0x240
[ 13.507507][81411103] __driver_attach+0xa3/0xb0
[ 13.507508]
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Just tested this patch on a macbook pro with i915 and radeon. First I get a nasty lockdep splat, then it locks up completely: [ 13.507373] fb: conflicting fb hw usage radeondrmfb vs EFI VGA - removing generic driver The EFI driver is known to need other work of its own. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, Jan 12, 2013 at 01:13:23PM -0800, Andrew Morton wrote: > Florian has been taking a month or two's downtime (now expired, I > think) so I've been waiting for him to reappear to process this one for > 3.8. > > Meanwhile, I guess we could put it into mainline ;) It has been in > -next for a month. > > From: Alan Cox > Subject: fb: rework locking to fix lock ordering on takeover > > Adjust the console layer to allow a take over call where the caller > already holds the locks. Make the fb layer lock in order. > > This is partly a band aid, the fb layer is terminally confused about the > locking rules it uses for its notifiers it seems. > > [a...@linux-foundation.org: remove stray non-ascii char, tidy comment] > [a...@linux-foundation.org: export do_take_over_console()] > Signed-off-by: Alan Cox > Cc: Florian Tobias Schandinat > Cc: Stephen Rothwell > Signed-off-by: Andrew Morton Yes, that's the one, modulo Andrew's fixes, which I've been running. Irregardless, I'll run this one tomorrow just in case, because it triggers here so easily. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, 12 Jan 2013 12:51:49 -0800 Linus Torvalds
wrote:
> On Sat, Jan 12, 2013 at 10:36 AM, Borislav Petkov wrote:
> > On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote:
> >> Still seeing it with current Linus' tree.
> >>
> >> Tested-by: Jiri Kosina
> >>
> >> Anyone applying this, please?
> >
> > Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch
> > from https://patchwork.kernel.org/patch/1757061/ please?
>
> Christ people.
>
> I already reported that it DOES NOT EVEN COMPILE. See the other thread
> here on lkml ("Re: 3.8-rc2: EFI framebuffer lock inversion...").
>
> Alan apparently doesn't care about the patch he wrote to even bother
> fixing that, and the only person who does seem to care enough to carry
> two fixes around (Andrew) apparently doesn't feel that he's
> comfortable forwarding it to me (he's been sending other patches, so
> it's not like Andrew is offline either)..
>
> I'm not picking up random patches from people who don't care enough
> about those patches to even bother fixing compile errors when
> reportyed and didn't even send them to me to begin with.
>
> So I'm trusting that Andrew is right, and is waiting for something.
>
Florian has been taking a month or two's downtime (now expired, I
think) so I've been waiting for him to reappear to process this one for
3.8.
Meanwhile, I guess we could put it into mainline ;) It has been in
-next for a month.
From: Alan Cox
Subject: fb: rework locking to fix lock ordering on takeover
Adjust the console layer to allow a take over call where the caller
already holds the locks. Make the fb layer lock in order.
This is partly a band aid, the fb layer is terminally confused about the
locking rules it uses for its notifiers it seems.
[a...@linux-foundation.org: remove stray non-ascii char, tidy comment]
[a...@linux-foundation.org: export do_take_over_console()]
Signed-off-by: Alan Cox
Cc: Florian Tobias Schandinat
Cc: Stephen Rothwell
Signed-off-by: Andrew Morton
---
drivers/tty/vt/vt.c | 91
drivers/video/console/fbcon.c | 30 ++
drivers/video/fbmem.c |5 -
drivers/video/fbsysfs.c |3 +
include/linux/console.h |1
5 files changed, 104 insertions(+), 26 deletions(-)
diff -puN
drivers/tty/vt/vt.c~fb-rework-locking-to-fix-lock-ordering-on-takeover
drivers/tty/vt/vt.c
--- a/drivers/tty/vt/vt.c~fb-rework-locking-to-fix-lock-ordering-on-takeover
+++ a/drivers/tty/vt/vt.c
@@ -2987,7 +2987,7 @@ int __init vty_init(const struct file_op
static struct class *vtconsole_class;
-static int bind_con_driver(const struct consw *csw, int first, int last,
+static int do_bind_con_driver(const struct consw *csw, int first, int last,
int deflt)
{
struct module *owner = csw->owner;
@@ -2998,7 +2998,7 @@ static int bind_con_driver(const struct
if (!try_module_get(owner))
return -ENODEV;
- console_lock();
+ WARN_CONSOLE_UNLOCKED();
/* check if driver is registered */
for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
@@ -3083,11 +3083,22 @@ static int bind_con_driver(const struct
retval = 0;
err:
- console_unlock();
module_put(owner);
return retval;
};
+
+static int bind_con_driver(const struct consw *csw, int first, int last,
+ int deflt)
+{
+ int ret;
+
+ console_lock();
+ ret = do_bind_con_driver(csw, first, last, deflt);
+ console_unlock();
+ return ret;
+}
+
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
static int con_is_graphics(const struct consw *csw, int first, int last)
{
@@ -3199,9 +3210,9 @@ int unbind_con_driver(const struct consw
if (!con_is_bound(csw))
con_driver->flag &= ~CON_DRIVER_FLAG_INIT;
- console_unlock();
/* ignore return value, binding should not fail */
- bind_con_driver(defcsw, first, last, deflt);
+ do_bind_con_driver(defcsw, first, last, deflt);
+ console_unlock();
err:
module_put(owner);
return retval;
@@ -3492,28 +3503,18 @@ int con_debug_leave(void)
}
EXPORT_SYMBOL_GPL(con_debug_leave);
-/**
- * register_con_driver - register console driver to console layer
- * @csw: console driver
- * @first: the first console to take over, minimum value is 0
- * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
- *
- * DESCRIPTION: This function registers a console driver which can later
- * bind to a range of consoles specified by @first and @last. It will
- * also initialize the console driver by calling con_startup().
- */
-int register_con_driver(const struct consw *csw, int first, int last)
+static int do_register_con_driver(const struct consw *csw, int first, int last)
{
struct module *owner = csw->owner;
struct con_driver *con_driver;
const char *desc;
int i, retval = 0;
+
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Alan is dealing with family stuff that is far more important. Someone who has the time needs to own this and the fb layer. Not a don't care - a don't have time. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, Jan 12, 2013 at 10:36 AM, Borislav Petkov wrote:
> On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote:
>> Still seeing it with current Linus' tree.
>>
>> Tested-by: Jiri Kosina
>>
>> Anyone applying this, please?
>
> Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch
> from https://patchwork.kernel.org/patch/1757061/ please?
Christ people.
I already reported that it DOES NOT EVEN COMPILE. See the other thread
here on lkml ("Re: 3.8-rc2: EFI framebuffer lock inversion...").
Alan apparently doesn't care about the patch he wrote to even bother
fixing that, and the only person who does seem to care enough to carry
two fixes around (Andrew) apparently doesn't feel that he's
comfortable forwarding it to me (he's been sending other patches, so
it's not like Andrew is offline either)..
I'm not picking up random patches from people who don't care enough
about those patches to even bother fixing compile errors when
reportyed and didn't even send them to me to begin with.
So I'm trusting that Andrew is right, and is waiting for something.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote: > Still seeing it with current Linus' tree. > > Tested-by: Jiri Kosina > > Anyone applying this, please? Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch from https://patchwork.kernel.org/patch/1757061/ please? Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote: Still seeing it with current Linus' tree. Tested-by: Jiri Kosina jkos...@suse.cz Anyone applying this, please? Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch from https://patchwork.kernel.org/patch/1757061/ please? Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, Jan 12, 2013 at 10:36 AM, Borislav Petkov b...@alien8.de wrote: On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote: Still seeing it with current Linus' tree. Tested-by: Jiri Kosina jkos...@suse.cz Anyone applying this, please? Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch from https://patchwork.kernel.org/patch/1757061/ please? Christ people. I already reported that it DOES NOT EVEN COMPILE. See the other thread here on lkml (Re: 3.8-rc2: EFI framebuffer lock inversion...). Alan apparently doesn't care about the patch he wrote to even bother fixing that, and the only person who does seem to care enough to carry two fixes around (Andrew) apparently doesn't feel that he's comfortable forwarding it to me (he's been sending other patches, so it's not like Andrew is offline either).. I'm not picking up random patches from people who don't care enough about those patches to even bother fixing compile errors when reportyed and didn't even send them to me to begin with. So I'm trusting that Andrew is right, and is waiting for something. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Alan is dealing with family stuff that is far more important. Someone who has the time needs to own this and the fb layer. Not a don't care - a don't have time. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, 12 Jan 2013 12:51:49 -0800 Linus Torvalds
torva...@linux-foundation.org wrote:
On Sat, Jan 12, 2013 at 10:36 AM, Borislav Petkov b...@alien8.de wrote:
On Mon, Jan 07, 2013 at 10:37:15AM +0100, Jiri Kosina wrote:
Still seeing it with current Linus' tree.
Tested-by: Jiri Kosina jkos...@suse.cz
Anyone applying this, please?
Yeah, I'm still seeing it in -rc3. Linus, can you pick up Alan's patch
from https://patchwork.kernel.org/patch/1757061/ please?
Christ people.
I already reported that it DOES NOT EVEN COMPILE. See the other thread
here on lkml (Re: 3.8-rc2: EFI framebuffer lock inversion...).
Alan apparently doesn't care about the patch he wrote to even bother
fixing that, and the only person who does seem to care enough to carry
two fixes around (Andrew) apparently doesn't feel that he's
comfortable forwarding it to me (he's been sending other patches, so
it's not like Andrew is offline either)..
I'm not picking up random patches from people who don't care enough
about those patches to even bother fixing compile errors when
reportyed and didn't even send them to me to begin with.
So I'm trusting that Andrew is right, and is waiting for something.
Florian has been taking a month or two's downtime (now expired, I
think) so I've been waiting for him to reappear to process this one for
3.8.
Meanwhile, I guess we could put it into mainline ;) It has been in
-next for a month.
From: Alan Cox a...@linux.intel.com
Subject: fb: rework locking to fix lock ordering on takeover
Adjust the console layer to allow a take over call where the caller
already holds the locks. Make the fb layer lock in order.
This is partly a band aid, the fb layer is terminally confused about the
locking rules it uses for its notifiers it seems.
[a...@linux-foundation.org: remove stray non-ascii char, tidy comment]
[a...@linux-foundation.org: export do_take_over_console()]
Signed-off-by: Alan Cox a...@linux.intel.com
Cc: Florian Tobias Schandinat florianschandi...@gmx.de
Cc: Stephen Rothwell s...@canb.auug.org.au
Signed-off-by: Andrew Morton a...@linux-foundation.org
---
drivers/tty/vt/vt.c | 91
drivers/video/console/fbcon.c | 30 ++
drivers/video/fbmem.c |5 -
drivers/video/fbsysfs.c |3 +
include/linux/console.h |1
5 files changed, 104 insertions(+), 26 deletions(-)
diff -puN
drivers/tty/vt/vt.c~fb-rework-locking-to-fix-lock-ordering-on-takeover
drivers/tty/vt/vt.c
--- a/drivers/tty/vt/vt.c~fb-rework-locking-to-fix-lock-ordering-on-takeover
+++ a/drivers/tty/vt/vt.c
@@ -2987,7 +2987,7 @@ int __init vty_init(const struct file_op
static struct class *vtconsole_class;
-static int bind_con_driver(const struct consw *csw, int first, int last,
+static int do_bind_con_driver(const struct consw *csw, int first, int last,
int deflt)
{
struct module *owner = csw-owner;
@@ -2998,7 +2998,7 @@ static int bind_con_driver(const struct
if (!try_module_get(owner))
return -ENODEV;
- console_lock();
+ WARN_CONSOLE_UNLOCKED();
/* check if driver is registered */
for (i = 0; i MAX_NR_CON_DRIVER; i++) {
@@ -3083,11 +3083,22 @@ static int bind_con_driver(const struct
retval = 0;
err:
- console_unlock();
module_put(owner);
return retval;
};
+
+static int bind_con_driver(const struct consw *csw, int first, int last,
+ int deflt)
+{
+ int ret;
+
+ console_lock();
+ ret = do_bind_con_driver(csw, first, last, deflt);
+ console_unlock();
+ return ret;
+}
+
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
static int con_is_graphics(const struct consw *csw, int first, int last)
{
@@ -3199,9 +3210,9 @@ int unbind_con_driver(const struct consw
if (!con_is_bound(csw))
con_driver-flag = ~CON_DRIVER_FLAG_INIT;
- console_unlock();
/* ignore return value, binding should not fail */
- bind_con_driver(defcsw, first, last, deflt);
+ do_bind_con_driver(defcsw, first, last, deflt);
+ console_unlock();
err:
module_put(owner);
return retval;
@@ -3492,28 +3503,18 @@ int con_debug_leave(void)
}
EXPORT_SYMBOL_GPL(con_debug_leave);
-/**
- * register_con_driver - register console driver to console layer
- * @csw: console driver
- * @first: the first console to take over, minimum value is 0
- * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
- *
- * DESCRIPTION: This function registers a console driver which can later
- * bind to a range of consoles specified by @first and @last. It will
- * also initialize the console driver by calling con_startup().
- */
-int register_con_driver(const struct consw *csw, int first, int last)
+static int do_register_con_driver(const struct consw *csw, int first, int last)
{
struct module *owner =
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Sat, Jan 12, 2013 at 01:13:23PM -0800, Andrew Morton wrote: Florian has been taking a month or two's downtime (now expired, I think) so I've been waiting for him to reappear to process this one for 3.8. Meanwhile, I guess we could put it into mainline ;) It has been in -next for a month. From: Alan Cox a...@linux.intel.com Subject: fb: rework locking to fix lock ordering on takeover Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This is partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. [a...@linux-foundation.org: remove stray non-ascii char, tidy comment] [a...@linux-foundation.org: export do_take_over_console()] Signed-off-by: Alan Cox a...@linux.intel.com Cc: Florian Tobias Schandinat florianschandi...@gmx.de Cc: Stephen Rothwell s...@canb.auug.org.au Signed-off-by: Andrew Morton a...@linux-foundation.org Yes, that's the one, modulo Andrew's fixes, which I've been running. Irregardless, I'll run this one tomorrow just in case, because it triggers here so easily. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, 28 Dec 2012, Borislav Petkov wrote: > > http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 > > Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, > good. > > Linus, Alan's patch works at least in 2 cases, you might consider > picking it up directly since the fb maintainer is absent, reportedly. > > Tested-by: Borislav Petkov Still seeing it with current Linus' tree. Tested-by: Jiri Kosina Anyone applying this, please? -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, 28 Dec 2012, Borislav Petkov wrote: http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Tested-by: Borislav Petkov b...@alien8.de Still seeing it with current Linus' tree. Tested-by: Jiri Kosina jkos...@suse.cz Anyone applying this, please? -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 05.01.2013 13:07, schrieb Alan Cox: So to add such an "I am crap" flag my idea would be to add an .fb_handle_damage to struct fb_ops and then call that (if exists) whenever something was changed. I was thinking much higher level - ie at the printk kind of level My patch (for udlfb) follows as an reply to this message. If that patch is ok, it should be applied to smscufx too (I would make it). In regard to udl I don't know, I haven't had a deeper look at it nor used it up to now. Looks pretty clean as a solution to me. Thanks and sorry for the two empty lines in the patch. I swear I had a look at the patch before sending it out, but haven't seen them. So should I make the same patch for smscufx and while beeing there, send out at v2 without those 2 empty lines? Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
> So to add such an "I am crap" flag my idea would be to add an > .fb_handle_damage to struct fb_ops and then call that (if exists) > whenever something was changed. I was thinking much higher level - ie at the printk kind of level > My patch (for udlfb) follows as an reply to this message. If that patch > is ok, it should be applied to smscufx too (I would make it). In regard > to udl I don't know, I haven't had a deeper look at it nor used it up to > now. Looks pretty clean as a solution to me. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 04.01.2013 14:36, schrieb Alexander Holler: Am 04.01.2013 14:25, schrieb Alan Cox: On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler wrote: ... Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the "Rework locking patch): They are broken if used as the system console (has been known for years). Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't That is what I wanted to try next. ;) want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console "I am crap" flag might be better than hacking each driver. All those drivers look very similiar. I will see if I'm successfull in writing such an IamCrapHelper. Might need some time, but I will post a patch for review, if I've done and tested it. I'm only using the USB-LCD on occasion, so it doesn't have high priority for me because I don't really need it. I've just added a work queue for dlfb_handle_damage. Up to now I've only tested the console, seems to work without any problems (even with lock checking on). In regard to that "I am crap" handler, I'm not sure how to do that. Just queuing the ops wherever they are used outside the drivers doesn't work, because e.g. if(i_am_crap) queue_work(ops.fb_imageblit(..., image)) else ops.fb_imageblit(..., image) doesn't work, because e.g. image will become destroyed before the work gets executed. And copying the whole image doesn't make sense. handle_damage() in contrast just needs the coordinates for a rectangle (x, y, w, h). So to add such an "I am crap" flag my idea would be to add an .fb_handle_damage to struct fb_ops and then call that (if exists) whenever something was changed. But I don't like that very much. I think that might end up in more changes than just changing those 3 very similiar drivers (I'm not sure if the queuing is needed for udl at all). Maybe it would make sense, to unify the stuff in those 3 similiar drivers moving the shared functions to one file which is used by them all. udl seems to have already split some stuff into different files. My patch (for udlfb) follows as an reply to this message. If that patch is ok, it should be applied to smscufx too (I would make it). In regard to udl I don't know, I haven't had a deeper look at it nor used it up to now. Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 04.01.2013 14:36, schrieb Alexander Holler: Am 04.01.2013 14:25, schrieb Alan Cox: On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler hol...@ahsoftware.de wrote: ... Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the Rework locking patch): They are broken if used as the system console (has been known for years). Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't That is what I wanted to try next. ;) want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console I am crap flag might be better than hacking each driver. All those drivers look very similiar. I will see if I'm successfull in writing such an IamCrapHelper. Might need some time, but I will post a patch for review, if I've done and tested it. I'm only using the USB-LCD on occasion, so it doesn't have high priority for me because I don't really need it. I've just added a work queue for dlfb_handle_damage. Up to now I've only tested the console, seems to work without any problems (even with lock checking on). In regard to that I am crap handler, I'm not sure how to do that. Just queuing the ops wherever they are used outside the drivers doesn't work, because e.g. if(i_am_crap) queue_work(ops.fb_imageblit(..., image)) else ops.fb_imageblit(..., image) doesn't work, because e.g. image will become destroyed before the work gets executed. And copying the whole image doesn't make sense. handle_damage() in contrast just needs the coordinates for a rectangle (x, y, w, h). So to add such an I am crap flag my idea would be to add an .fb_handle_damage to struct fb_ops and then call that (if exists) whenever something was changed. But I don't like that very much. I think that might end up in more changes than just changing those 3 very similiar drivers (I'm not sure if the queuing is needed for udl at all). Maybe it would make sense, to unify the stuff in those 3 similiar drivers moving the shared functions to one file which is used by them all. udl seems to have already split some stuff into different files. My patch (for udlfb) follows as an reply to this message. If that patch is ok, it should be applied to smscufx too (I would make it). In regard to udl I don't know, I haven't had a deeper look at it nor used it up to now. Regards, Alexander -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
So to add such an I am crap flag my idea would be to add an .fb_handle_damage to struct fb_ops and then call that (if exists) whenever something was changed. I was thinking much higher level - ie at the printk kind of level My patch (for udlfb) follows as an reply to this message. If that patch is ok, it should be applied to smscufx too (I would make it). In regard to udl I don't know, I haven't had a deeper look at it nor used it up to now. Looks pretty clean as a solution to me. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 05.01.2013 13:07, schrieb Alan Cox: So to add such an I am crap flag my idea would be to add an .fb_handle_damage to struct fb_ops and then call that (if exists) whenever something was changed. I was thinking much higher level - ie at the printk kind of level My patch (for udlfb) follows as an reply to this message. If that patch is ok, it should be applied to smscufx too (I would make it). In regard to udl I don't know, I haven't had a deeper look at it nor used it up to now. Looks pretty clean as a solution to me. Thanks and sorry for the two empty lines in the patch. I swear I had a look at the patch before sending it out, but haven't seen them. So should I make the same patch for smscufx and while beeing there, send out at v2 without those 2 empty lines? Regards, Alexander -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 04.01.2013 14:25, schrieb Alan Cox: On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler wrote: Am 28.12.2012 13:40, schrieb Borislav Petkov: On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the "Rework locking patch): They are broken if used as the system console (has been known for years). Ah. Thats why I didn't see it before. Usually I've used the serial as system console. So thats why it worked before. ;) Perhaps your x86 test has the system console still on another device ? Exactly thats the case. Thanks for pointing it out. For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver for udl which obsoletes the old fb layer one and works much better (although the error handling is still totally broken and leaks like a sieve if it fails) Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't That is what I wanted to try next. ;) want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console "I am crap" flag might be better than hacking each driver. All those drivers look very similiar. I will see if I'm successfull in writing such an IamCrapHelper. Might need some time, but I will post a patch for review, if I've done and tested it. I'm only using the USB-LCD on occasion, so it doesn't have high priority for me because I don't really need it. Thanks for the hints. Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler wrote: > Am 28.12.2012 13:40, schrieb Borislav Petkov: > > On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: > >> +1 > >> > >> http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 > > > > Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, > > good. > > > > Linus, Alan's patch works at least in 2 cases, you might consider > > picking it up directly since the fb maintainer is absent, reportedly. > > > Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at > least on ARM(v5). When I have linked in udlfb the following happens on > boot (with an attached USB-LCD and with or without the "Rework locking > patch): They are broken if used as the system console (has been known for years). Perhaps your x86 test has the system console still on another device ? For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver for udl which obsoletes the old fb layer one and works much better (although the error handling is still totally broken and leaks like a sieve if it fails) Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console "I am crap" flag might be better than hacking each driver. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 28.12.2012 13:40, schrieb Borislav Petkov: On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the "Rework locking patch): dlfb_init_framebuffer_work() (work) register_framebuffer() (lock mutex registration_lock) (vt_console_print) (spinlock printing_lock) (fbcon_scroll) (fbcon_redraw) (fbcon_putcs) (bit_putcs) dlfb_ops_imageblit() dlfb_handle_damage() dlfb_get_urb() down_timeout(semaphore) BUG: scheduling while atomic (vt_console_print) (release spinlock printing_lock) register_framebuffer() (unlock mutex registration_lock) The above is without the "Rework locking" patch. But I get the same BUG with the patch (so the patch doesn't do any harm), I just haven't looked in detail what changed with the patch. I don't get the BUG when I try the same on a x86_64 machine, not sure why. I've just started to read me through the code, documentation and the "Rework locking" patch. And I'm slow in doing so (spare time). But maybe someone else with more knowledge about the inner workings of this stuff is faster than I. Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 28.12.2012 13:40, schrieb Borislav Petkov: On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the Rework locking patch): dlfb_init_framebuffer_work() (work) register_framebuffer() (lock mutex registration_lock) (vt_console_print) (spinlock printing_lock) (fbcon_scroll) (fbcon_redraw) (fbcon_putcs) (bit_putcs) dlfb_ops_imageblit() dlfb_handle_damage() dlfb_get_urb() down_timeout(semaphore) BUG: scheduling while atomic (vt_console_print) (release spinlock printing_lock) register_framebuffer() (unlock mutex registration_lock) The above is without the Rework locking patch. But I get the same BUG with the patch (so the patch doesn't do any harm), I just haven't looked in detail what changed with the patch. I don't get the BUG when I try the same on a x86_64 machine, not sure why. I've just started to read me through the code, documentation and the Rework locking patch. And I'm slow in doing so (spare time). But maybe someone else with more knowledge about the inner workings of this stuff is faster than I. Regards, Alexander -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler hol...@ahsoftware.de wrote: Am 28.12.2012 13:40, schrieb Borislav Petkov: On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the Rework locking patch): They are broken if used as the system console (has been known for years). Perhaps your x86 test has the system console still on another device ? For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver for udl which obsoletes the old fb layer one and works much better (although the error handling is still totally broken and leaks like a sieve if it fails) Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console I am crap flag might be better than hacking each driver. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
Am 04.01.2013 14:25, schrieb Alan Cox: On Fri, 04 Jan 2013 13:50:37 +0100 Alexander Holler hol...@ahsoftware.de wrote: Am 28.12.2012 13:40, schrieb Borislav Petkov: On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Btw. I think all the usb-fb's (udlfb, smscufx and udl) are broken, at least on ARM(v5). When I have linked in udlfb the following happens on boot (with an attached USB-LCD and with or without the Rework locking patch): They are broken if used as the system console (has been known for years). Ah. Thats why I didn't see it before. Usually I've used the serial as system console. So thats why it worked before. ;) Perhaps your x86 test has the system console still on another device ? Exactly thats the case. Thanks for pointing it out. For the udl layer it shouldn't matter as Dave Airlie wrote a DRM driver for udl which obsoletes the old fb layer one and works much better (although the error handling is still totally broken and leaks like a sieve if it fails) Fixing the console isn't that difficult - you just need to make your device queue the console I/O to a worker thread of some kind. We don't That is what I wanted to try next. ;) want to do that by default because we want to get the messages out reliably and immediately on saner hardware. Given there are several such cases a general helper and a console I am crap flag might be better than hacking each driver. All those drivers look very similiar. I will see if I'm successfull in writing such an IamCrapHelper. Might need some time, but I will post a patch for review, if I've done and tested it. I'm only using the USB-LCD on occasion, so it doesn't have high priority for me because I don't really need it. Thanks for the hints. Regards, Alexander -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: > +1 > > http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Tested-by: Borislav Petkov Thanks. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Thu, Dec 27, 2012 at 05:53:01AM +0100, Borislav Petkov wrote: > On Wed, Dec 26, 2012 at 01:09:51PM -0500, Sasha Levin wrote: > > > This patch can fix the following warning we saw? > > > http://lkml.org/lkml/2012/12/22/53 > > > > > > I will give it a try. > > > > Yup, that's the same error I've reported couple of months ago. > > > > It looks like the fb maintains are still absent, so it'll probably > > need a different way to get upstream. > > Adding to the bug pressure: just got a very similar splat on -rc1 (see > below). Alan, I'll run your patch to verify. > +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Shawn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Thu, Dec 27, 2012 at 05:53:01AM +0100, Borislav Petkov wrote: On Wed, Dec 26, 2012 at 01:09:51PM -0500, Sasha Levin wrote: This patch can fix the following warning we saw? http://lkml.org/lkml/2012/12/22/53 I will give it a try. Yup, that's the same error I've reported couple of months ago. It looks like the fb maintains are still absent, so it'll probably need a different way to get upstream. Adding to the bug pressure: just got a very similar splat on -rc1 (see below). Alan, I'll run your patch to verify. +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Shawn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, Dec 28, 2012 at 07:50:27PM +0800, Shawn Guo wrote: +1 http://thread.gmane.org/gmane.linux.kernel/1413953/focus=1415070 Cool, works fine here too. Is Linus on CC? (/me checks.. ) Yes he is, good. Linus, Alan's patch works at least in 2 cases, you might consider picking it up directly since the fb maintainer is absent, reportedly. Tested-by: Borislav Petkov b...@alien8.de Thanks. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Dec 26, 2012 at 01:09:51PM -0500, Sasha Levin wrote:
> > This patch can fix the following warning we saw?
> > http://lkml.org/lkml/2012/12/22/53
> >
> > I will give it a try.
>
> Yup, that's the same error I've reported couple of months ago.
>
> It looks like the fb maintains are still absent, so it'll probably
> need a different way to get upstream.
Adding to the bug pressure: just got a very similar splat on -rc1 (see
below). Alan, I'll run your patch to verify.
Thanks.
[33946.663968] ==
[33946.663970] [ INFO: possible circular locking dependency detected ]
[33946.663978] 3.8.0-rc1+ #1 Not tainted
[33946.663980] ---
[33946.663986] kworker/1:2/15780 is trying to acquire lock:
[33946.664010] ((fb_notifier_list).rwsem){.+}, at: []
__blocking_notifier_call_chain+0x33/0x60
[33946.664013]
[33946.664013] but task is already holding lock:
[33946.664029] (console_lock){+.+.+.}, at: []
console_callback+0x13/0x160
[33946.664032]
[33946.664032] which lock already depends on the new lock.
[33946.664032]
[33946.664034]
[33946.664034] the existing dependency chain (in reverse order) is:
[33946.664042]
[33946.664042] -> #1 (console_lock){+.+.+.}:
[33946.664054][] lock_acquire+0x8a/0x140
[33946.664063][] console_lock+0x5f/0x70
[33946.664072][] register_con_driver+0x39/0x150
[33946.664080][] take_over_console+0x2e/0x70
[33946.664088][] fbcon_takeover+0x5a/0xb0
[33946.664096][] fbcon_event_notify+0x5eb/0x6f0
[33946.664103][] notifier_call_chain+0x4c/0x70
[33946.664111][]
__blocking_notifier_call_chain+0x4b/0x60
[33946.664119][]
blocking_notifier_call_chain+0x16/0x20
[33946.664127][] fb_notifier_call_chain+0x1b/0x20
[33946.664136][] register_framebuffer+0x1bc/0x2f0
[33946.664169][]
drm_fb_helper_single_fb_probe+0x1e3/0x310 [drm_kms_helper]
[33946.664183][]
drm_fb_helper_initial_config+0x1d1/0x230 [drm_kms_helper]
[33946.664239][] radeon_fbdev_init+0xc1/0x120 [radeon]
[33946.664290][] radeon_modeset_init+0x3a8/0xb90
[radeon]
[33946.664333][] radeon_driver_load_kms+0xf0/0x180
[radeon]
[33946.664344][] drm_get_pci_dev+0x186/0x2d0
[33946.664379][] radeon_pci_probe+0xb3/0xf0 [radeon]
[33946.664390][] pci_device_probe+0x9c/0xe0
[33946.664400][] driver_probe_device+0x8b/0x3a0
[33946.664408][] __driver_attach+0xab/0xb0
[33946.664415][] bus_for_each_dev+0x55/0x90
[33946.664422][] driver_attach+0x1e/0x20
[33946.664429][] bus_add_driver+0x1b0/0x2a0
[33946.664437][] driver_register+0x77/0x160
[33946.664445][] __pci_register_driver+0x64/0x70
[33946.664452][] drm_pci_init+0x10c/0x120
[33946.664480][] inet6_ioctl+0x7/0xb0 [ipv6]
[33946.664491][] do_one_initcall+0x122/0x170
[33946.664500][] load_module+0x185f/0x2160
[33946.664507][] sys_init_module+0xae/0x110
[33946.664516][] system_call_fastpath+0x16/0x1b
[33946.664526]
[33946.664526] -> #0 ((fb_notifier_list).rwsem){.+}:
[33946.664534][] __lock_acquire+0x1ae8/0x1b10
[33946.664542][] lock_acquire+0x8a/0x140
[33946.664549][] down_read+0x34/0x49
[33946.664557][]
__blocking_notifier_call_chain+0x33/0x60
[33946.664564][]
blocking_notifier_call_chain+0x16/0x20
[33946.664572][] fb_notifier_call_chain+0x1b/0x20
[33946.664579][] fb_blank+0x3b/0xc0
[33946.664586][] fbcon_blank+0x223/0x2d0
[33946.664595][] do_blank_screen+0x1cb/0x270
[33946.664603][] console_callback+0x6a/0x160
[33946.664612][] process_one_work+0x19d/0x5e0
[33946.664620][] worker_thread+0x15d/0x450
[33946.664628][] kthread+0xea/0xf0
[33946.664636][] ret_from_fork+0x7c/0xb0
[33946.664638]
[33946.664638] other info that might help us debug this:
[33946.664638]
[33946.664641] Possible unsafe locking scenario:
[33946.664641]
[33946.664643]CPU0CPU1
[33946.664645]
[33946.664650] lock(console_lock);
[33946.664656]lock((fb_notifier_list).rwsem);
[33946.664661]lock(console_lock);
[33946.664666] lock((fb_notifier_list).rwsem);
[33946.664667]
[33946.664667] *** DEADLOCK ***
[33946.664667]
[33946.664671] 3 locks held by kworker/1:2/15780:
[33946.664686] #0: (events){.+.+.+}, at: []
process_one_work+0x130/0x5e0
[33946.664701] #1: (console_work){+.+.+.}, at: []
process_one_work+0x130/0x5e0
[33946.664715] #2: (console_lock){+.+.+.}, at: []
console_callback+0x13/0x160
[33946.664717]
[33946.664717] stack backtrace:
[33946.664723] Pid: 15780, comm: kworker/1:2 Not tainted 3.8.0-rc1+ #1
[33946.664726] Call Trace:
[33946.664736] [] print_circular_bug+0x1fe/0x20f
[33946.664745] []
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Tue, Dec 25, 2012 at 9:41 PM, Cong Wang wrote: > On Wed, Dec 26, 2012 at 12:08 AM, Sasha Levin wrote: >> On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer wrote: >>> On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox wrote: On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer wrote: > On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox > wrote: > > > > [The fb maintainer appears to be absent at the moment]. > > > > This is needed to fix a pile of lockdep splats that now show up because > > console_lock() > > is being properly audited. Hugh Dickins and Sasha Levin have tested it > > and both reports > > all looks good. This is probably not the whole story - the entire fb > > layer has locking > > confusion problems that were previously hidden but it seems to get the > > ones people hit > > in testing. This hopefully explains a few of the weird fb hangs that > > have been floating > > around forever. > > > > From: Alan Cox > > > > Adjust the console layer to allow a take over call where the caller > > already > > holds the locks. Make the fb layer lock in order. > > > > This s partly a band aid, the fb layer is terminally confused about the > > locking rules it uses for its notifiers it seems. > > > > Signed-off-by: Alan Cox > > Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. >>> >>> So... this patch seems to still be twisting in the wind. It should >>> probably be headed into 3.8 at this point, shouldn't it? >> >> Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have >> to carry this patch to avoid them. > > This patch can fix the following warning we saw? > http://lkml.org/lkml/2012/12/22/53 > > I will give it a try. Yup, that's the same error I've reported couple of months ago. It looks like the fb maintains are still absent, so it'll probably need a different way to get upstream. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Tue, Dec 25, 2012 at 9:41 PM, Cong Wang xiyou.wangc...@gmail.com wrote: On Wed, Dec 26, 2012 at 12:08 AM, Sasha Levin levinsasha...@gmail.com wrote: On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer jwbo...@gmail.com wrote: On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer jwbo...@gmail.com wrote: On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. So... this patch seems to still be twisting in the wind. It should probably be headed into 3.8 at this point, shouldn't it? Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have to carry this patch to avoid them. This patch can fix the following warning we saw? http://lkml.org/lkml/2012/12/22/53 I will give it a try. Yup, that's the same error I've reported couple of months ago. It looks like the fb maintains are still absent, so it'll probably need a different way to get upstream. Thanks, Sasha -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Dec 26, 2012 at 01:09:51PM -0500, Sasha Levin wrote:
This patch can fix the following warning we saw?
http://lkml.org/lkml/2012/12/22/53
I will give it a try.
Yup, that's the same error I've reported couple of months ago.
It looks like the fb maintains are still absent, so it'll probably
need a different way to get upstream.
Adding to the bug pressure: just got a very similar splat on -rc1 (see
below). Alan, I'll run your patch to verify.
Thanks.
[33946.663968] ==
[33946.663970] [ INFO: possible circular locking dependency detected ]
[33946.663978] 3.8.0-rc1+ #1 Not tainted
[33946.663980] ---
[33946.663986] kworker/1:2/15780 is trying to acquire lock:
[33946.664010] ((fb_notifier_list).rwsem){.+}, at: [810776a3]
__blocking_notifier_call_chain+0x33/0x60
[33946.664013]
[33946.664013] but task is already holding lock:
[33946.664029] (console_lock){+.+.+.}, at: [812ecda3]
console_callback+0x13/0x160
[33946.664032]
[33946.664032] which lock already depends on the new lock.
[33946.664032]
[33946.664034]
[33946.664034] the existing dependency chain (in reverse order) is:
[33946.664042]
[33946.664042] - #1 (console_lock){+.+.+.}:
[33946.664054][810a821a] lock_acquire+0x8a/0x140
[33946.664063][8104bf5f] console_lock+0x5f/0x70
[33946.664072][812e94f9] register_con_driver+0x39/0x150
[33946.664080][812eafde] take_over_console+0x2e/0x70
[33946.664088][8128409a] fbcon_takeover+0x5a/0xb0
[33946.664096][81287c5b] fbcon_event_notify+0x5eb/0x6f0
[33946.664103][8107758c] notifier_call_chain+0x4c/0x70
[33946.664111][810776bb]
__blocking_notifier_call_chain+0x4b/0x60
[33946.664119][810776e6]
blocking_notifier_call_chain+0x16/0x20
[33946.664127][8127903b] fb_notifier_call_chain+0x1b/0x20
[33946.664136][8127a76c] register_framebuffer+0x1bc/0x2f0
[33946.664169][a00e0283]
drm_fb_helper_single_fb_probe+0x1e3/0x310 [drm_kms_helper]
[33946.664183][a00e0581]
drm_fb_helper_initial_config+0x1d1/0x230 [drm_kms_helper]
[33946.664239][a04a74a1] radeon_fbdev_init+0xc1/0x120 [radeon]
[33946.664290][a04a22b8] radeon_modeset_init+0x3a8/0xb90
[radeon]
[33946.664333][a047f0e0] radeon_driver_load_kms+0xf0/0x180
[radeon]
[33946.664344][81314bd6] drm_get_pci_dev+0x186/0x2d0
[33946.664379][a0465183] radeon_pci_probe+0xb3/0xf0 [radeon]
[33946.664390][8126687c] pci_device_probe+0x9c/0xe0
[33946.664400][8132cf6b] driver_probe_device+0x8b/0x3a0
[33946.664408][8132d32b] __driver_attach+0xab/0xb0
[33946.664415][8132aea5] bus_for_each_dev+0x55/0x90
[33946.664422][8132c9ae] driver_attach+0x1e/0x20
[33946.664429][8132c500] bus_add_driver+0x1b0/0x2a0
[33946.664437][8132da17] driver_register+0x77/0x160
[33946.664445][81265724] __pci_register_driver+0x64/0x70
[33946.664452][81314e2c] drm_pci_init+0x10c/0x120
[33946.664480][a05470e7] inet6_ioctl+0x7/0xb0 [ipv6]
[33946.664491][810002f2] do_one_initcall+0x122/0x170
[33946.664500][810b520f] load_module+0x185f/0x2160
[33946.664507][810b5bbe] sys_init_module+0xae/0x110
[33946.664516][814c7f42] system_call_fastpath+0x16/0x1b
[33946.664526]
[33946.664526] - #0 ((fb_notifier_list).rwsem){.+}:
[33946.664534][810a7c88] __lock_acquire+0x1ae8/0x1b10
[33946.664542][810a821a] lock_acquire+0x8a/0x140
[33946.664549][814c4ba4] down_read+0x34/0x49
[33946.664557][810776a3]
__blocking_notifier_call_chain+0x33/0x60
[33946.664564][810776e6]
blocking_notifier_call_chain+0x16/0x20
[33946.664572][8127903b] fb_notifier_call_chain+0x1b/0x20
[33946.664579][812797cb] fb_blank+0x3b/0xc0
[33946.664586][81287f83] fbcon_blank+0x223/0x2d0
[33946.664595][812ebd6b] do_blank_screen+0x1cb/0x270
[33946.664603][812ecdfa] console_callback+0x6a/0x160
[33946.664612][81068f0d] process_one_work+0x19d/0x5e0
[33946.664620][8106a7dd] worker_thread+0x15d/0x450
[33946.664628][81070d5a] kthread+0xea/0xf0
[33946.664636][814c7e9c] ret_from_fork+0x7c/0xb0
[33946.664638]
[33946.664638] other info that might help us debug this:
[33946.664638]
[33946.664641] Possible unsafe locking scenario:
[33946.664641]
[33946.664643]CPU0CPU1
[33946.664645]
[33946.664650] lock(console_lock);
[33946.664656]
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Dec 26, 2012 at 12:08 AM, Sasha Levin wrote: > On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer wrote: >> On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox wrote: >>> On Wed, 21 Nov 2012 07:45:45 -0500 >>> Josh Boyer wrote: >>> On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox wrote: > > [The fb maintainer appears to be absent at the moment]. > > This is needed to fix a pile of lockdep splats that now show up because > console_lock() > is being properly audited. Hugh Dickins and Sasha Levin have tested it > and both reports > all looks good. This is probably not the whole story - the entire fb > layer has locking > confusion problems that were previously hidden but it seems to get the > ones people hit > in testing. This hopefully explains a few of the weird fb hangs that > have been floating > around forever. > > From: Alan Cox > > Adjust the console layer to allow a take over call where the caller > already > holds the locks. Make the fb layer lock in order. > > This s partly a band aid, the fb layer is terminally confused about the > locking rules it uses for its notifiers it seems. > > Signed-off-by: Alan Cox Should this eventually get into the stable trees? >>> >>> Thats a question I'm not sure about at this point. I think the bug is >>> real but not caught by the lock checker in older trees but I've not >>> investigated. >> >> So... this patch seems to still be twisting in the wind. It should >> probably be headed into 3.8 at this point, shouldn't it? > > Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have > to carry this patch to avoid them. This patch can fix the following warning we saw? http://lkml.org/lkml/2012/12/22/53 I will give it a try. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer wrote: > On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox wrote: >> On Wed, 21 Nov 2012 07:45:45 -0500 >> Josh Boyer wrote: >> >>> On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox wrote: >>> > >>> > [The fb maintainer appears to be absent at the moment]. >>> > >>> > This is needed to fix a pile of lockdep splats that now show up because >>> > console_lock() >>> > is being properly audited. Hugh Dickins and Sasha Levin have tested it >>> > and both reports >>> > all looks good. This is probably not the whole story - the entire fb >>> > layer has locking >>> > confusion problems that were previously hidden but it seems to get the >>> > ones people hit >>> > in testing. This hopefully explains a few of the weird fb hangs that have >>> > been floating >>> > around forever. >>> > >>> > From: Alan Cox >>> > >>> > Adjust the console layer to allow a take over call where the caller >>> > already >>> > holds the locks. Make the fb layer lock in order. >>> > >>> > This s partly a band aid, the fb layer is terminally confused about the >>> > locking rules it uses for its notifiers it seems. >>> > >>> > Signed-off-by: Alan Cox >>> >>> Should this eventually get into the stable trees? >> >> Thats a question I'm not sure about at this point. I think the bug is >> real but not caught by the lock checker in older trees but I've not >> investigated. > > So... this patch seems to still be twisting in the wind. It should > probably be headed into 3.8 at this point, shouldn't it? Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have to carry this patch to avoid them. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer jwbo...@gmail.com wrote: On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer jwbo...@gmail.com wrote: On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. So... this patch seems to still be twisting in the wind. It should probably be headed into 3.8 at this point, shouldn't it? Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have to carry this patch to avoid them. Thanks, Sasha -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Dec 26, 2012 at 12:08 AM, Sasha Levin levinsasha...@gmail.com wrote: On Tue, Dec 18, 2012 at 10:20 AM, Josh Boyer jwbo...@gmail.com wrote: On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer jwbo...@gmail.com wrote: On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. So... this patch seems to still be twisting in the wind. It should probably be headed into 3.8 at this point, shouldn't it? Indeed it should. I'm seeing the original warnings in 3.8-rc1 and have to carry this patch to avoid them. This patch can fix the following warning we saw? http://lkml.org/lkml/2012/12/22/53 I will give it a try. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox wrote: > On Wed, 21 Nov 2012 07:45:45 -0500 > Josh Boyer wrote: > >> On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox wrote: >> > >> > [The fb maintainer appears to be absent at the moment]. >> > >> > This is needed to fix a pile of lockdep splats that now show up because >> > console_lock() >> > is being properly audited. Hugh Dickins and Sasha Levin have tested it and >> > both reports >> > all looks good. This is probably not the whole story - the entire fb layer >> > has locking >> > confusion problems that were previously hidden but it seems to get the >> > ones people hit >> > in testing. This hopefully explains a few of the weird fb hangs that have >> > been floating >> > around forever. >> > >> > From: Alan Cox >> > >> > Adjust the console layer to allow a take over call where the caller already >> > holds the locks. Make the fb layer lock in order. >> > >> > This s partly a band aid, the fb layer is terminally confused about the >> > locking rules it uses for its notifiers it seems. >> > >> > Signed-off-by: Alan Cox >> >> Should this eventually get into the stable trees? > > Thats a question I'm not sure about at this point. I think the bug is > real but not caught by the lock checker in older trees but I've not > investigated. So... this patch seems to still be twisting in the wind. It should probably be headed into 3.8 at this point, shouldn't it? josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, Nov 21, 2012 at 7:53 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer jwbo...@gmail.com wrote: On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. So... this patch seems to still be twisting in the wind. It should probably be headed into 3.8 at this point, shouldn't it? josh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer wrote: > On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox wrote: > > > > [The fb maintainer appears to be absent at the moment]. > > > > This is needed to fix a pile of lockdep splats that now show up because > > console_lock() > > is being properly audited. Hugh Dickins and Sasha Levin have tested it and > > both reports > > all looks good. This is probably not the whole story - the entire fb layer > > has locking > > confusion problems that were previously hidden but it seems to get the ones > > people hit > > in testing. This hopefully explains a few of the weird fb hangs that have > > been floating > > around forever. > > > > From: Alan Cox > > > > Adjust the console layer to allow a take over call where the caller already > > holds the locks. Make the fb layer lock in order. > > > > This s partly a band aid, the fb layer is terminally confused about the > > locking rules it uses for its notifiers it seems. > > > > Signed-off-by: Alan Cox > > Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox wrote: > > [The fb maintainer appears to be absent at the moment]. > > This is needed to fix a pile of lockdep splats that now show up because > console_lock() > is being properly audited. Hugh Dickins and Sasha Levin have tested it and > both reports > all looks good. This is probably not the whole story - the entire fb layer > has locking > confusion problems that were previously hidden but it seems to get the ones > people hit > in testing. This hopefully explains a few of the weird fb hangs that have > been floating > around forever. > > From: Alan Cox > > Adjust the console layer to allow a take over call where the caller already > holds the locks. Make the fb layer lock in order. > > This s partly a band aid, the fb layer is terminally confused about the > locking rules it uses for its notifiers it seems. > > Signed-off-by: Alan Cox Should this eventually get into the stable trees? josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? josh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fb: Rework locking to fix lock ordering on takeover
On Wed, 21 Nov 2012 07:45:45 -0500 Josh Boyer jwbo...@gmail.com wrote: On Fri, Nov 16, 2012 at 2:27 PM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: [The fb maintainer appears to be absent at the moment]. This is needed to fix a pile of lockdep splats that now show up because console_lock() is being properly audited. Hugh Dickins and Sasha Levin have tested it and both reports all looks good. This is probably not the whole story - the entire fb layer has locking confusion problems that were previously hidden but it seems to get the ones people hit in testing. This hopefully explains a few of the weird fb hangs that have been floating around forever. From: Alan Cox a...@linux.intel.com Adjust the console layer to allow a take over call where the caller already holds the locks. Make the fb layer lock in order. This s partly a band aid, the fb layer is terminally confused about the locking rules it uses for its notifiers it seems. Signed-off-by: Alan Cox a...@linux.intel.com Should this eventually get into the stable trees? Thats a question I'm not sure about at this point. I think the bug is real but not caught by the lock checker in older trees but I've not investigated. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
