[git pull] fbcon locking fixes.

[git pull] fbcon locking fixes.

[Dave Airlie]

Hi Linus,

These patches have been sailing around long enough, waiting for a maintainer
to reappear, so I've decided enough is enough, lockdep is kinda useful to have.

Thanks to Daniel for annoying me enough :-)

Dave 'not the fbdev maintainer' Airlie.

The following changes since commit ba2ab41f3d68f277860768b2c5b197768b196332:

  Merge tag 'pm+acpi-for-3.8-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm (2013-01-24 10:19:13 -0800)

are available in the git repository at:


  git://people.freedesktop.org/~airlied/linux fbcon-locking-fixes

for you to fetch changes up to a9d2331a8eccc17408abd5a7cd3e463745254d65:

  fb: Yet another band-aid for fixing lockdep mess (2013-01-25 10:34:49 +1000)

----------------------------------------------------------------
Alan Cox (1):
      fb: rework locking to fix lock ordering on takeover

Takashi Iwai (1):
      fb: Yet another band-aid for fixing lockdep mess

 drivers/tty/vt/vt.c           | 134 ++++++++++++++++++++++++++++++------------
 drivers/video/console/fbcon.c |  33 ++++++++++-
 drivers/video/fbmem.c         |   9 ++-
 drivers/video/fbsysfs.c       |   3 +
 include/linux/console.h       |   2 +
 include/linux/vt_kern.h       |   2 +
 6 files changed, 140 insertions(+), 43 deletions(-)

Re: [git pull] fbcon locking fixes.

[Andrew Morton]

On Fri, 25 Jan 2013 00:42:37 +0000 (GMT)
Dave Airlie <airlied@linux.ie> wrote:

> These patches have been sailing around long enough, waiting for a maintainer
> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
> 
> Thanks to Daniel for annoying me enough :-)

Me too, but the patches broke Maarten's EFI driver setup:
https://lkml.org/lkml/2013/1/15/203.

Re: [git pull] fbcon locking fixes.

[Daniel Vetter]

On Fri, Jan 25, 2013 at 1:50 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 25 Jan 2013 00:42:37 +0000 (GMT)
> Dave Airlie <airlied@linux.ie> wrote:
>
>> These patches have been sailing around long enough, waiting for a maintainer
>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>
>> Thanks to Daniel for annoying me enough :-)
>
> Me too, but the patches broke Maarten's EFI driver setup:
> https://lkml.org/lkml/2013/1/15/203.

Oops, didn't know that there are open issues, otherwise I'd have
worked on fixes instead of annoying you guys. To make due I've crawled
through fbcon code a bit to hunt for ways we could solve this mess
better than just flinging duct-tape. All the patches extend the
console_lock'ed area quite a bit, and I don't really like big kernel
looks with creeping coverage. More so if they come with funny
semantics like console_lock attached.

Looking at the fb notifier there are three use-cases for it:
- Special blank/unblank handling, used by the backlight and lcd
drivers in the fbdev subsystem. Could easily be extracted to another
notifier chain, and seems to have no need for the console lock. Also,
for kms drivers I don't care about this one bit.
- suspend/resume handling for the same fbdev lcd drivers. Same applies
(besides that it probably should get converted to proper device model
suspend/resume stuff).
- Runtime dependency injection between fbdev (and vga_switcheroor) for
fbcon. This allows you to load fbcon.ko and fbdev drives in any order,
and end up with an fbcon on each fbdev. Or not load fbcon.ko at all,
and only use the underlying fbdev. On a quick look it also racy as
hell.

The last one is the tricky one. If we could just add a hard
(compile-time) dependency between fbdevs and fbcon, we could rip out
all the fb notifier madness which also involves the console_lock in
some way and should be able to straighten out the locking. Users could
still opt out of using the fbcon at runtime, but ofc this might break
a really obscure setup somewhere. Quick survey of distros shows though
that all use the sane option of built-in fbcon support.

My proposal is to change the fbcon tristate to a bool and see what
happens. If it works out for a few kernel releases, garbage-collect
the fb_notifier for fbcon and use direct function calls instead
(stubbed it out properly if fbcon is disabeld ofc). Or is that too
much bending of the "thou shalt not break stuff" rule?

Plan B would be to simply mash-up a console on top of kms drivers
directly (since that's what I care about). Which has the downside that
we'd still need to keep the drm fbdev emulation helper code around,
since quite a few people use that to run older userspace (and even
report some bugs, e.g. mplayer on the linux console played some tricks
we needed to catch). And so end up with two pieces of code who's main
job is to paint console output onto the screen.

Ideas?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [git pull] fbcon locking fixes.

[Linus Torvalds]

On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>
> These patches have been sailing around long enough, waiting for a maintainer
> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.

Last this was tried, these patches failed miserably.

They caused instant lockdep splat and then a total lockup with efifb.
It may be that Takashi's patch helps fix that problem, but it's in no
way clear that it does, so the patch series isn't at all obviously
stable.

Yes, lockdep is indeed "kinda useful", and there clearly are locking
problems in fbdev. But I'm not seeing myself pulling these for 3.8.
They've been too problematic to pull in at this late stage.

              Linus

Re: [git pull] fbcon locking fixes.

[Dave Airlie]

On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>
>> These patches have been sailing around long enough, waiting for a maintainer
>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>
> Last this was tried, these patches failed miserably.
>
> They caused instant lockdep splat and then a total lockup with efifb.
> It may be that Takashi's patch helps fix that problem, but it's in no
> way clear that it does, so the patch series isn't at all obviously
> stable.
>
> Yes, lockdep is indeed "kinda useful", and there clearly are locking
> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
> They've been too problematic to pull in at this late stage.
>

Okay I'll fix the efifb problem and then maybe queue them for -next.

Dave.

Re: [git pull] fbcon locking fixes.

[Dave Airlie]

On Fri, Jan 25, 2013 at 11:06 AM, Dave Airlie <airlied@gmail.com> wrote:
> On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>>
>>> These patches have been sailing around long enough, waiting for a maintainer
>>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>
>> Last this was tried, these patches failed miserably.
>>
>> They caused instant lockdep splat and then a total lockup with efifb.
>> It may be that Takashi's patch helps fix that problem, but it's in no
>> way clear that it does, so the patch series isn't at all obviously
>> stable.
>>
>> Yes, lockdep is indeed "kinda useful", and there clearly are locking
>> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
>> They've been too problematic to pull in at this late stage.
>>
>
> Okay I'll fix the efifb problem and then maybe queue them for -next.

Okay I've just sent out another fbcon patch to fix the locking harder.

There was a path going into set_con2fb_path if an fb driver was
already registered, I just pushed the locking out further on anyone
going in there.

it boots on my EFI macbook here.

Dave.

Re: [git pull] fbcon locking fixes.

[Linus Torvalds]

On Thu, Jan 24, 2013 at 5:45 PM, Dave Airlie <airlied@gmail.com> wrote:
>
> Okay I've just sent out another fbcon patch to fix the locking harder.
>
> There was a path going into set_con2fb_path if an fb driver was
> already registered, I just pushed the locking out further on anyone
> going in there.
>
> it boots on my EFI macbook here.

Ok, good. Sounds like we'll finally get it fixed, but I'm still too
much of a scaredy-cat to take it for now, so -next it is...

              Linus

Re: [git pull] fbcon locking fixes.

[Maarten Lankhorst]

Hey,

Op 25-01-13 02:45, Dave Airlie schreef:
> On Fri, Jan 25, 2013 at 11:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>> On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>>> These patches have been sailing around long enough, waiting for a maintainer
>>>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>> Last this was tried, these patches failed miserably.
>>>
>>> They caused instant lockdep splat and then a total lockup with efifb.
>>> It may be that Takashi's patch helps fix that problem, but it's in no
>>> way clear that it does, so the patch series isn't at all obviously
>>> stable.
>>>
>>> Yes, lockdep is indeed "kinda useful", and there clearly are locking
>>> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
>>> They've been too problematic to pull in at this late stage.
>>>
>> Okay I'll fix the efifb problem and then maybe queue them for -next.
> Okay I've just sent out another fbcon patch to fix the locking harder.
>
> There was a path going into set_con2fb_path if an fb driver was
> already registered, I just pushed the locking out further on anyone
> going in there.
>
> it boots on my EFI macbook here.
>
I cherry picked those patches to my tree, and the full series no longer triggers a lockdep warning.
It also no longer locks up during modprobing or vga-switcheroo either.

Tested-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

~Maarten

Re: [git pull] fbcon locking fixes.

[Daniel Vetter]

On Mon, Jan 28, 2013 at 1:45 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
>> There was a path going into set_con2fb_path if an fb driver was
>> already registered, I just pushed the locking out further on anyone
>> going in there.
>>
>> it boots on my EFI macbook here.
>>
> I cherry picked those patches to my tree, and the full series no longer triggers a lockdep warning.
> It also no longer locks up during modprobing or vga-switcheroo either.
>
> Tested-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

QA reported spurious failures in some kms_flip tests and lockdep
splats, somehow both are fixed with the locking. I'm a bit confused
how these issues could cause failures in the flip tests, but alas:

Tested-by: Lu Hua <huax.lu@intel.com> (for all three patches).
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch