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(-)
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.
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
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
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.
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.
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
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
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).
dri-devel@lists.freedesktop.org
-
Andrew Morton -
Daniel Vetter -
Dave Airlie -
Dave Airlie -
Linus Torvalds -
Maarten Lankhorst