Re: [patch] splice mmap_sem deadlock

[Balbir Singh <balbir@linux.vnet.ibm.com>]

Linus Torvalds wrote:
> 
> On Mon, 1 Oct 2007, Balbir Singh wrote:
>> Sounds very similar to the problems we had with CPU hotplug earlier.
>> It's a rwlock locking anti-pattern. I know that recursive locks
>> have been frowned upon earlier, but I wonder if there is a case here.
>> Of-course recursive locks would not be *fair*.
> 
> The problem with recursive locks is that they are inevitably done wrong.
> 
> For example, the "natural" way  to do them is to just save the process ID 
> or something like that. Which is utter crap. Yet, people do it *every* 
> single time (yes, I've done it too, I admit).
> 

Yes, I've done that too, I guess we learn what's bad by doing it.

> The thing is, "recursive" doesn't mean "same CPU" or "same process" or 
> "same thread" or anything like that. It means "same *dependency-chain*". 
> With the very real implication that you literally have to pass the "lock 
> instance" (whether that is a cookie or anything else) around, and thus 
> really generate the proper chain.
> 
> For example, in CPU hotplug, the dependency chain really did end up moving 
> between different execution contexts, iirc (eg from process context into 
> kernel workqueues).
> 

I agree with whatever you've said so far. The original intention of
every lock is to protect data not code. In the example mentioned before
and in the case of CPU hotplug, what we intend to do, is to prevent
the writer from causing a deadlock. We create a deadlock, as a
side-effect of the locking is fair.

I guess what we might need is a variant of unfair locks. Tracking owners
is one way of eliminating deadlocks that occur, specially since the
reader-writer lock does not allow readers to provide fair locking.
I think this is where RCU excels, it allows readers/writers to proceed
almost independently.

> So we could add some kind of recursive interface that maintained a list of 
> ownership or whatever, but the fact remains that after 16 years, we still 
> haven't really needed it, except for code that is so ugly and broken that 
> pretty much everybody really feels it should be rewritten (and generally 
> for *other* reasons) anyway.
> 

Recursive mutexes are meant for a special purpose. Uncontrolled use of
recursive locks would be really bad.

> So I'm not categorically against nesting, but I'm certainly down on it, 
> and I think it's almost always done wrong.
> 

Yes, recursive locking needs to be designed with care and usage reviewed
with a lot of attention.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>