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

2015-12-08 Thread 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-12-07 Thread Tomi Valkeinen 

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



signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-24 Thread Tomi Valkeinen 


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



signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-01 Thread Tomi Valkeinen 


On 25/08/15 22:24, Rob Clark wrote:
> On Tue, Aug 25, 2015 at 9:45 AM, 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.
>>
>> Cc: Jean-Christophe Plagniol-Villard 
>> Cc: Tomi Valkeinen 
>> Cc: linux-fb...@vger.kernel.org
>> Signed-off-by: Daniel Vetter 
> 
> 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



signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-01 Thread Rob Clark 
On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen  wrote:
>
>
> On 25/08/15 22:24, Rob Clark wrote:
>> On Tue, Aug 25, 2015 at 9:45 AM, 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.
>>>
>>> Cc: Jean-Christophe Plagniol-Villard 
>>> Cc: Tomi Valkeinen 
>>> Cc: linux-fb...@vger.kernel.org
>>> Signed-off-by: Daniel Vetter 
>>
>> 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
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-01 Thread Rob Clark 
On Tue, Sep 1, 2015 at 10:41 AM, Tomi Valkeinen  wrote:
>
>
> On 01/09/15 17:34, Rob Clark wrote:
>> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen  wrote:
>>>
>>>
>>> On 25/08/15 22:24, Rob Clark wrote:
 On Tue, Aug 25, 2015 at 9:45 AM, 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.
>
> Cc: Jean-Christophe Plagniol-Villard 
> Cc: Tomi Valkeinen 
> Cc: linux-fb...@vger.kernel.org
> Signed-off-by: Daniel Vetter 

 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
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-01 Thread Tomi Valkeinen 


On 01/09/15 17:34, Rob Clark wrote:
> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen  wrote:
>>
>>
>> On 25/08/15 22:24, Rob Clark wrote:
>>> On Tue, Aug 25, 2015 at 9:45 AM, 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.

 Cc: Jean-Christophe Plagniol-Villard 
 Cc: Tomi Valkeinen 
 Cc: linux-fb...@vger.kernel.org
 Signed-off-by: Daniel Vetter 
>>>
>>> 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



signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-09-01 Thread 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  wrote:
> >
> >
> > On 01/09/15 17:34, Rob Clark wrote:
> >> On Tue, Sep 1, 2015 at 6:32 AM, Tomi Valkeinen  
> >> wrote:
> >>>
> >>>
> >>> On 25/08/15 22:24, Rob Clark wrote:
>  On Tue, Aug 25, 2015 at 9:45 AM, 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.
> >
> > Cc: Jean-Christophe Plagniol-Villard 
> > Cc: Tomi Valkeinen 
> > Cc: linux-fb...@vger.kernel.org
> > Signed-off-by: Daniel Vetter 
> 
>  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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2015-08-25 Thread Rob Clark 
On Tue, Aug 25, 2015 at 9:45 AM, Daniel Vetter daniel.vet...@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 plagn...@jcrosoft.com
 Cc: Tomi Valkeinen tomi.valkei...@ti.com
 Cc: linux-fb...@vger.kernel.org
 Signed-off-by: Daniel Vetter daniel.vet...@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-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx