Re: [PATCH v2 2/3] lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE

From: Matthew Wilcox
Date: Thu Oct 19 2017 - 16:33:22 EST


On Thu, Oct 19, 2017 at 08:21:56PM +0000, Bart Van Assche wrote:
> In case it wouldn't be clear, your work and the work of others on lockdep
> and preempt-rt is highly appreciated. Sorry that I missed the discussion
> about the cross-release functionality when it went upstream. I have several
> questions about that functionality:
> * How many lock inversion problems have been found so far thanks to the
> cross-release checking? How many false positives have the cross-release
> checks triggered so far? Does the number of real issues that has been
> found outweigh the effort spent on suppressing false positives?
> * What alternatives have been considered other than enabling cross-release
> checking for all locking objects that support releasing from the context
> of another task than the context from which the lock was obtained? Has it
> e.g. been considered to introduce two versions of the lock objects that
> support cross-releases - one version for which lock inversion checking is
> always enabled and another version for which lock inversion checking is
> always disabled?
> * How much review has the Documentation/locking/crossrelease.txt received
> before it went upstream? At least to me that document seems much harder
> to read than other kernel documentation due to weird use of the English
> grammar.

While interesting, I think this list of questions misses an important one:

* How many bugs is this going to catch in the future?

For example, the page lock is not annotatable with lockdep -- we return
to userspace with it held, for heaven's sake! So it is quite easy for
someone not familiar with the MM locking hierarchy to inadvertently
introduce an ABBA deadlock against the page lock. (ie me. I did that.)
Right now, that has to be caught by a human reviewer; if cross-release
checking can catch that, then it's worth having.