Actually, this painful dependency may technically be a bug in drm, but at
the same time, it's really filldir() that makes it _very_ hard for certain
locking scenarios because it has that callback to the VFS layer that takes
the mmap_sem, and sysfs itself wants the sysfs_mutex in paths where it is
not at all unreasonable to hold the mmap_sem.
We've seen it several times (yes, mostly with drm, but it's been seen with
others too), and it's very annoying. It can be fixed by having very
careful readdir implementations, but I really would blame sysfs in
particular for having a very annoying lock reversal issue when used
reasonably.
So the optimal situation would be for sysfs to not have that annoying lock
dependency, and it would really have to be sysfs_readdir() that drops the
sysfs_mutex around the filldir call (and that obviously implies having to
re-validate and be really careful).
Added Eric and Greg to the cc, in case the sysfs people want to solve it.
The other alternative would be to fix this on a VFS layer by changing how
'readdir/filldir' interacts, and instead make it fill in a kernel buffer -
avoiding the mmap_sem issue entirely. And than later (in readdir) that
kernel buffer could be copied to user space without holding any other
locks.
I like the VFS approach because I think we could possibly use that as a
first approach to eventually try to think about caching readdir() results
at a VFS level - readdir() is currently the _only_ main filesystem
callback that always calls into the low-level filesystem, and always takes
a lot of locks. I'm adding Al to the Cc for that - he knows about this
issue from me previously thinking aloud along these lines.
And yes, one option would be to just fix drm - by avoiding calling any
sysfs functions while holding the mmap_lock (either in the mmap callback
or the page fault paths). However, as mentioned, I really do think that
the blame can be laid on sysfs for trying to be a nice generic interface,
but having a damn inconvenient locking model.
Linus
--
