Re: [PATCH] break_lock forever broken

[Ingo Molnar <mingo@elte.hu>]

* Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> > while writing the ->break_lock feature i intentionally avoided
> > overhead in the spinlock fastpath. A better solution for the bug you
> > noticed is to clear the break_lock flag in places that use
> > need_lock_break() explicitly.
> 
> What happens if break_lock gets set by random contention on the lock
> somewhere (with no need_lock_break or cond_resched_lock)? Next time it
> goes through a lockbreak will (may) be a false positive.

yes, and that's harmless. Lock contention is supposed to be a relatively
rare thing (compared to the frequency of uncontended locking), so all
the overhead is concentrated towards the contention case, not towards
the uncontended case. If the flag lingers then it may be a false
positive and the lock will be dropped once, the flag will be cleared,
and the lock will be reacquired. So we've traded a constant amount of
overhead in the fastpath for a somewhat higher, but still constant
amount of overhead in the slowpath.

> >One robust way for that seems to be to make the need_lock_break() macro
> >clear the flag if it sees it set, and to make all the other (internal)
> >users use __need_lock_break() that doesnt clear the flag. I'll cook up a
> >patch for this.
> >
> 
> If you do this exactly as you describe, then you'll break
> cond_resched_lock (eg. for the copy_page_range path), won't you?

(cond_resched_lock() is an 'internal' user that will use
__need_lock_break().)

	Ingo