[PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Daniel Vetter]

When the usual fbcon legacy options are enabled we have
->register_framebuffer
  ->fb notifier chain calls into fbcon
    ->fbcon sets up console on new fbi
      ->fbi->set_par
        ->drm_fb_helper_set_par exercises full kms api

And because of locking inversion hilarity all of register_framebuffer
is done with the console lock held. Which means that the first time on
driver load we exercise _all_ the kms code (all probe paths and
modeset paths for everything connected) is under the console lock.
That means if anything goes belly-up in that big pile of code nothing
ever reaches logfiles (and the machine is dead).

Usual tactic to debug that is to temporarily remove those console_lock
calls to be able to capture backtraces. I'm fed up writing this patch
and recompiling kernels. Hence this patch here to add an unsafe,
kernel-taining option to do this at runtime.

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/video/fbdev/core/fbmem.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0705d8883ede..4e73b6f6b1c0 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1608,6 +1608,11 @@ static int do_remove_conflicting_framebuffers(struct apertures_struct *a,
 	return 0;
 }
 
+static bool lockless_register_fb;
+module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400);
+MODULE_PARM_DESC(lockless_register_fb,
+	"Lockless framebuffer registration for debugging [default=off]");
+
 static int do_register_framebuffer(struct fb_info *fb_info)
 {
 	int i, ret;
@@ -1675,15 +1680,18 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	registered_fb[i] = fb_info;
 
 	event.info = fb_info;
-	console_lock();
+	if (!lockless_register_fb)
+		console_lock();
 	if (!lock_fb_info(fb_info)) {
-		console_unlock();
+		if (!lockless_register_fb)
+			console_unlock();
 		return -ENODEV;
 	}
 
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
 	unlock_fb_info(fb_info);
-	console_unlock();
+	if (!lockless_register_fb)
+		console_unlock();
 	return 0;
 }
 
-- 
1.8.3.1

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Rob Clark]

On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> When the usual fbcon legacy options are enabled we have
> ->register_framebuffer
>   ->fb notifier chain calls into fbcon
>     ->fbcon sets up console on new fbi
>       ->fbi->set_par
>         ->drm_fb_helper_set_par exercises full kms api
>
> And because of locking inversion hilarity all of register_framebuffer
> is done with the console lock held. Which means that the first time on
> driver load we exercise _all_ the kms code (all probe paths and
> modeset paths for everything connected) is under the console lock.
> That means if anything goes belly-up in that big pile of code nothing
> ever reaches logfiles (and the machine is dead).
>
> Usual tactic to debug that is to temporarily remove those console_lock
> calls to be able to capture backtraces. I'm fed up writing this patch
> and recompiling kernels. Hence this patch here to add an unsafe,
> kernel-taining option to do this at runtime.
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

This one was causing me some problems, if I tried to enable
lockless_register_fb.  It *looks* like it should work, so I'm not
quite sure what the deal is.  But I'm 110% fan of getting something
like this working, because console_lock is pretty much the bane of kms
developer's existence..

I'll have to debug further on a system where I can see more than the
bottom three lines of the second to last backtrace..

BR,
-R

> ---
>  drivers/video/fbdev/core/fbmem.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 0705d8883ede..4e73b6f6b1c0 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1608,6 +1608,11 @@ static int do_remove_conflicting_framebuffers(struct apertures_struct *a,
>         return 0;
>  }
>
> +static bool lockless_register_fb;
> +module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400);
> +MODULE_PARM_DESC(lockless_register_fb,
> +       "Lockless framebuffer registration for debugging [default=off]");
> +
>  static int do_register_framebuffer(struct fb_info *fb_info)
>  {
>         int i, ret;
> @@ -1675,15 +1680,18 @@ static int do_register_framebuffer(struct fb_info *fb_info)
>         registered_fb[i] = fb_info;
>
>         event.info = fb_info;
> -       console_lock();
> +       if (!lockless_register_fb)
> +               console_lock();
>         if (!lock_fb_info(fb_info)) {
> -               console_unlock();
> +               if (!lockless_register_fb)
> +                       console_unlock();
>                 return -ENODEV;
>         }
>
>         fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
>         unlock_fb_info(fb_info);
> -       console_unlock();
> +       if (!lockless_register_fb)
> +               console_unlock();
>         return 0;
>  }
>
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]



On 25/08/15 22:24, Rob Clark wrote:
> On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> When the usual fbcon legacy options are enabled we have
>> ->register_framebuffer
>>   ->fb notifier chain calls into fbcon
>>     ->fbcon sets up console on new fbi
>>       ->fbi->set_par
>>         ->drm_fb_helper_set_par exercises full kms api
>>
>> And because of locking inversion hilarity all of register_framebuffer
>> is done with the console lock held. Which means that the first time on
>> driver load we exercise _all_ the kms code (all probe paths and
>> modeset paths for everything connected) is under the console lock.
>> That means if anything goes belly-up in that big pile of code nothing
>> ever reaches logfiles (and the machine is dead).
>>
>> Usual tactic to debug that is to temporarily remove those console_lock
>> calls to be able to capture backtraces. I'm fed up writing this patch
>> and recompiling kernels. Hence this patch here to add an unsafe,
>> kernel-taining option to do this at runtime.
>>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> This one was causing me some problems, if I tried to enable
> lockless_register_fb.  It *looks* like it should work, so I'm not
> quite sure what the deal is.  But I'm 110% fan of getting something
> like this working, because console_lock is pretty much the bane of kms
> developer's existence..
> 
> I'll have to debug further on a system where I can see more than the
> bottom three lines of the second to last backtrace..

Any idea if anyone has ever looked at properly fixing this?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Rob Clark]

On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 25/08/15 22:24, Rob Clark wrote:
>> On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> When the usual fbcon legacy options are enabled we have
>>> ->register_framebuffer
>>>   ->fb notifier chain calls into fbcon
>>>     ->fbcon sets up console on new fbi
>>>       ->fbi->set_par
>>>         ->drm_fb_helper_set_par exercises full kms api
>>>
>>> And because of locking inversion hilarity all of register_framebuffer
>>> is done with the console lock held. Which means that the first time on
>>> driver load we exercise _all_ the kms code (all probe paths and
>>> modeset paths for everything connected) is under the console lock.
>>> That means if anything goes belly-up in that big pile of code nothing
>>> ever reaches logfiles (and the machine is dead).
>>>
>>> Usual tactic to debug that is to temporarily remove those console_lock
>>> calls to be able to capture backtraces. I'm fed up writing this patch
>>> and recompiling kernels. Hence this patch here to add an unsafe,
>>> kernel-taining option to do this at runtime.
>>>
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: linux-fbdev@vger.kernel.org
>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> This one was causing me some problems, if I tried to enable
>> lockless_register_fb.  It *looks* like it should work, so I'm not
>> quite sure what the deal is.  But I'm 110% fan of getting something
>> like this working, because console_lock is pretty much the bane of kms
>> developer's existence..
>>
>> I'll have to debug further on a system where I can see more than the
>> bottom three lines of the second to last backtrace..
>
> Any idea if anyone has ever looked at properly fixing this?

I hadn't had a chance to look at it further yet..  I think Daniel
claimed it worked for him, but he was probably on intel-next, where I
was on drm-next at the time which seemed to be having some unrelated
i915 issues (when I was trying to debug atomic fb-helper patches).  So
can't really say that the issue I had was actually related to this
patch.  I'll try again later this week or next, when hopefully i915 in
drm-next is in better shape..

BR,
-R

>  Tomi
>

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]



On 01/09/15 17:34, Rob Clark wrote:
> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>>
>> On 25/08/15 22:24, Rob Clark wrote:
>>> On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>> When the usual fbcon legacy options are enabled we have
>>>> ->register_framebuffer
>>>>   ->fb notifier chain calls into fbcon
>>>>     ->fbcon sets up console on new fbi
>>>>       ->fbi->set_par
>>>>         ->drm_fb_helper_set_par exercises full kms api
>>>>
>>>> And because of locking inversion hilarity all of register_framebuffer
>>>> is done with the console lock held. Which means that the first time on
>>>> driver load we exercise _all_ the kms code (all probe paths and
>>>> modeset paths for everything connected) is under the console lock.
>>>> That means if anything goes belly-up in that big pile of code nothing
>>>> ever reaches logfiles (and the machine is dead).
>>>>
>>>> Usual tactic to debug that is to temporarily remove those console_lock
>>>> calls to be able to capture backtraces. I'm fed up writing this patch
>>>> and recompiling kernels. Hence this patch here to add an unsafe,
>>>> kernel-taining option to do this at runtime.
>>>>
>>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>> Cc: linux-fbdev@vger.kernel.org
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>> This one was causing me some problems, if I tried to enable
>>> lockless_register_fb.  It *looks* like it should work, so I'm not
>>> quite sure what the deal is.  But I'm 110% fan of getting something
>>> like this working, because console_lock is pretty much the bane of kms
>>> developer's existence..
>>>
>>> I'll have to debug further on a system where I can see more than the
>>> bottom three lines of the second to last backtrace..
>>
>> Any idea if anyone has ever looked at properly fixing this?
> 
> I hadn't had a chance to look at it further yet..  I think Daniel
> claimed it worked for him, but he was probably on intel-next, where I
> was on drm-next at the time which seemed to be having some unrelated
> i915 issues (when I was trying to debug atomic fb-helper patches).  So
> can't really say that the issue I had was actually related to this
> patch.  I'll try again later this week or next, when hopefully i915 in
> drm-next is in better shape..

Oh, I didn't mean this patch, but the whole console lock in general.
I've also banged my head to a wall because of it =).

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Rob Clark]

On Tue, Sep 1, 2015 at 10:41 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 01/09/15 17:34, Rob Clark wrote:
>> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>
>>>
>>> On 25/08/15 22:24, Rob Clark wrote:
>>>> On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>>> When the usual fbcon legacy options are enabled we have
>>>>> ->register_framebuffer
>>>>>   ->fb notifier chain calls into fbcon
>>>>>     ->fbcon sets up console on new fbi
>>>>>       ->fbi->set_par
>>>>>         ->drm_fb_helper_set_par exercises full kms api
>>>>>
>>>>> And because of locking inversion hilarity all of register_framebuffer
>>>>> is done with the console lock held. Which means that the first time on
>>>>> driver load we exercise _all_ the kms code (all probe paths and
>>>>> modeset paths for everything connected) is under the console lock.
>>>>> That means if anything goes belly-up in that big pile of code nothing
>>>>> ever reaches logfiles (and the machine is dead).
>>>>>
>>>>> Usual tactic to debug that is to temporarily remove those console_lock
>>>>> calls to be able to capture backtraces. I'm fed up writing this patch
>>>>> and recompiling kernels. Hence this patch here to add an unsafe,
>>>>> kernel-taining option to do this at runtime.
>>>>>
>>>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>>> Cc: linux-fbdev@vger.kernel.org
>>>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>
>>>> This one was causing me some problems, if I tried to enable
>>>> lockless_register_fb.  It *looks* like it should work, so I'm not
>>>> quite sure what the deal is.  But I'm 110% fan of getting something
>>>> like this working, because console_lock is pretty much the bane of kms
>>>> developer's existence..
>>>>
>>>> I'll have to debug further on a system where I can see more than the
>>>> bottom three lines of the second to last backtrace..
>>>
>>> Any idea if anyone has ever looked at properly fixing this?
>>
>> I hadn't had a chance to look at it further yet..  I think Daniel
>> claimed it worked for him, but he was probably on intel-next, where I
>> was on drm-next at the time which seemed to be having some unrelated
>> i915 issues (when I was trying to debug atomic fb-helper patches).  So
>> can't really say that the issue I had was actually related to this
>> patch.  I'll try again later this week or next, when hopefully i915 in
>> drm-next is in better shape..
>
> Oh, I didn't mean this patch, but the whole console lock in general.
> I've also banged my head to a wall because of it =).

oh, not sure.. every time I've started looking closer at
console/console_lock I run away screaming..  I guess if it were
possible to push the lock down further so only drivers that needed the
lock (presumably serial/net/etc) could take it, that would be nice..
but not sure I am that brave..

BR,
-R

>  Tomi
>

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Daniel Vetter]

On Tue, Sep 01, 2015 at 11:12:11AM -0400, Rob Clark wrote:
> On Tue, Sep 1, 2015 at 10:41 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> >
> > On 01/09/15 17:34, Rob Clark wrote:
> >> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >>>
> >>>
> >>> On 25/08/15 22:24, Rob Clark wrote:
> >>>> On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >>>>> When the usual fbcon legacy options are enabled we have
> >>>>> ->register_framebuffer
> >>>>>   ->fb notifier chain calls into fbcon
> >>>>>     ->fbcon sets up console on new fbi
> >>>>>       ->fbi->set_par
> >>>>>         ->drm_fb_helper_set_par exercises full kms api
> >>>>>
> >>>>> And because of locking inversion hilarity all of register_framebuffer
> >>>>> is done with the console lock held. Which means that the first time on
> >>>>> driver load we exercise _all_ the kms code (all probe paths and
> >>>>> modeset paths for everything connected) is under the console lock.
> >>>>> That means if anything goes belly-up in that big pile of code nothing
> >>>>> ever reaches logfiles (and the machine is dead).
> >>>>>
> >>>>> Usual tactic to debug that is to temporarily remove those console_lock
> >>>>> calls to be able to capture backtraces. I'm fed up writing this patch
> >>>>> and recompiling kernels. Hence this patch here to add an unsafe,
> >>>>> kernel-taining option to do this at runtime.
> >>>>>
> >>>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> >>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>>>> Cc: linux-fbdev@vger.kernel.org
> >>>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>>>
> >>>> This one was causing me some problems, if I tried to enable
> >>>> lockless_register_fb.  It *looks* like it should work, so I'm not
> >>>> quite sure what the deal is.  But I'm 110% fan of getting something
> >>>> like this working, because console_lock is pretty much the bane of kms
> >>>> developer's existence..
> >>>>
> >>>> I'll have to debug further on a system where I can see more than the
> >>>> bottom three lines of the second to last backtrace..
> >>>
> >>> Any idea if anyone has ever looked at properly fixing this?
> >>
> >> I hadn't had a chance to look at it further yet..  I think Daniel
> >> claimed it worked for him, but he was probably on intel-next, where I
> >> was on drm-next at the time which seemed to be having some unrelated
> >> i915 issues (when I was trying to debug atomic fb-helper patches).  So
> >> can't really say that the issue I had was actually related to this
> >> patch.  I'll try again later this week or next, when hopefully i915 in
> >> drm-next is in better shape..
> >
> > Oh, I didn't mean this patch, but the whole console lock in general.
> > I've also banged my head to a wall because of it =).
> 
> oh, not sure.. every time I've started looking closer at
> console/console_lock I run away screaming..  I guess if it were
> possible to push the lock down further so only drivers that needed the
> lock (presumably serial/net/etc) could take it, that would be nice..
> but not sure I am that brave..

console_lock is pretty much unfixable without rewriting half of fbdev.
Which I don't expect to ever happen. For the curious look at all the
commits changing locking in fbdev over the past few years.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]



On 01/09/15 17:34, Rob Clark wrote:

> I hadn't had a chance to look at it further yet..  I think Daniel
> claimed it worked for him, but he was probably on intel-next, where I
> was on drm-next at the time which seemed to be having some unrelated
> i915 issues (when I was trying to debug atomic fb-helper patches).  So
> can't really say that the issue I had was actually related to this
> patch.  I'll try again later this week or next, when hopefully i915 in
> drm-next is in better shape..

Rob, did you have a chance to test this?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]


On 25/08/15 16:45, Daniel Vetter wrote:
> When the usual fbcon legacy options are enabled we have
> ->register_framebuffer
>   ->fb notifier chain calls into fbcon
>     ->fbcon sets up console on new fbi
>       ->fbi->set_par
>         ->drm_fb_helper_set_par exercises full kms api
> 
> And because of locking inversion hilarity all of register_framebuffer
> is done with the console lock held. Which means that the first time on
> driver load we exercise _all_ the kms code (all probe paths and
> modeset paths for everything connected) is under the console lock.
> That means if anything goes belly-up in that big pile of code nothing
> ever reaches logfiles (and the machine is dead).
> 
> Usual tactic to debug that is to temporarily remove those console_lock
> calls to be able to capture backtraces. I'm fed up writing this patch
> and recompiling kernels. Hence this patch here to add an unsafe,
> kernel-taining option to do this at runtime.

I think this was never merged. This was part 4 of 4, were there
dependencies or...? Should I apply this for 4.5?

But then... I think my issues with console lock have been later at
runtime, not at register. Maybe we need a module option to disable the
console lock altogether. I wonder how much havoc that might create, though.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Daniel Vetter]

On Mon, Dec 07, 2015 at 07:32:42PM +0200, Tomi Valkeinen wrote:
> 
> On 25/08/15 16:45, Daniel Vetter wrote:
> > When the usual fbcon legacy options are enabled we have
> > ->register_framebuffer
> >   ->fb notifier chain calls into fbcon
> >     ->fbcon sets up console on new fbi
> >       ->fbi->set_par
> >         ->drm_fb_helper_set_par exercises full kms api
> > 
> > And because of locking inversion hilarity all of register_framebuffer
> > is done with the console lock held. Which means that the first time on
> > driver load we exercise _all_ the kms code (all probe paths and
> > modeset paths for everything connected) is under the console lock.
> > That means if anything goes belly-up in that big pile of code nothing
> > ever reaches logfiles (and the machine is dead).
> > 
> > Usual tactic to debug that is to temporarily remove those console_lock
> > calls to be able to capture backtraces. I'm fed up writing this patch
> > and recompiling kernels. Hence this patch here to add an unsafe,
> > kernel-taining option to do this at runtime.
> 
> I think this was never merged. This was part 4 of 4, were there
> dependencies or...? Should I apply this for 4.5?

Patches 1-3 have all already landed in drm, and patch 4 is free standing.
Would be great if you can pull it in.
 
> But then... I think my issues with console lock have been later at
> runtime, not at register. Maybe we need a module option to disable the
> console lock altogether. I wonder how much havoc that might create, though.

Hm, where in fbdev do you hold the console_lock outside of
registering/unregistering an fbdev (because of fbcon)? There's of course
general trouble with console_lock deadlocks and fun like that, but ime
that all got a lot more manageable since I added lockdep annotations to
console_lock.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 4/4] fbdev: Debug knob to register without holding console_lock

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]



On 08/12/15 10:19, Daniel Vetter wrote:
> On Mon, Dec 07, 2015 at 07:32:42PM +0200, Tomi Valkeinen wrote:
>>
>> On 25/08/15 16:45, Daniel Vetter wrote:
>>> When the usual fbcon legacy options are enabled we have
>>> ->register_framebuffer
>>>   ->fb notifier chain calls into fbcon
>>>     ->fbcon sets up console on new fbi
>>>       ->fbi->set_par
>>>         ->drm_fb_helper_set_par exercises full kms api
>>>
>>> And because of locking inversion hilarity all of register_framebuffer
>>> is done with the console lock held. Which means that the first time on
>>> driver load we exercise _all_ the kms code (all probe paths and
>>> modeset paths for everything connected) is under the console lock.
>>> That means if anything goes belly-up in that big pile of code nothing
>>> ever reaches logfiles (and the machine is dead).
>>>
>>> Usual tactic to debug that is to temporarily remove those console_lock
>>> calls to be able to capture backtraces. I'm fed up writing this patch
>>> and recompiling kernels. Hence this patch here to add an unsafe,
>>> kernel-taining option to do this at runtime.
>>
>> I think this was never merged. This was part 4 of 4, were there
>> dependencies or...? Should I apply this for 4.5?
> 
> Patches 1-3 have all already landed in drm, and patch 4 is free standing.
> Would be great if you can pull it in.

Ok, I'll apply for 4.5.

>> But then... I think my issues with console lock have been later at
>> runtime, not at register. Maybe we need a module option to disable the
>> console lock altogether. I wonder how much havoc that might create, though.
> 
> Hm, where in fbdev do you hold the console_lock outside of
> registering/unregistering an fbdev (because of fbcon)? There's of course
> general trouble with console_lock deadlocks and fun like that, but ime
> that all got a lot more manageable since I added lockdep annotations to
> console_lock.

I don't know... I just have a vague recollection that I was having
trouble with the lock with... crashes during blanking, perhaps. I really
can't remember, so possibly things are better now, or I just remember wrong.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]