Re: check && lockdep_no_validate (Was: lockdep: Introduce wait-typechecks)

[Alan Stern]

On Thu, 16 Jan 2014, Peter Zijlstra wrote:

> On Thu, Jan 16, 2014 at 06:43:48PM +0100, Oleg Nesterov wrote:
> > But with or without this change the following code
> > 
> > 		static DEFINE_MUTEX(m1);
> > 		static DEFINE_MUTEX(mx);
> > 
> > 		lockdep_set_novalidate_class(&mx);
> > 
> > 		// m1 -> mx
> > 		mutex_lock(&m1);
> > 		mutex_lock(&mx);
> > 		mutex_unlock(&mx);
> > 		mutex_unlock(&m1);
> > 
> > 		// mx -> m1 ; should trigger the warning ???
> > 		mutex_lock(&mx);
> > 		mutex_lock(&m1);
> > 		mutex_unlock(&m1);
> > 		mutex_unlock(&mx);
> > 
> > doesn't trigger the warning too. This is correct because
> > lockdep_set_novalidate_class() means, well, no-validate.
> > The question is: do we really want to avoid all validations?
> 
> Good question.
> 
> > Why lockdep_set_novalidate_class() was added? Unlees I missed
> > something the problem is that (say) __driver_attach() can take
> > the "same" lock twice, drivers/base/ lacks annotations.
> 
> Indeed, the driver model locking always slips my mind but yes its
> creative. Alan Stern seems to have a good grasp on it though.

Mostly my "grasp" is just a firm belief that trying to manage locking
throughout the entire driver tree is hopeless.  Individual sub-portions
of it are usually well behaved, but the thing as a whole is a mess.

> > Perhaps we should change the meaning of lockdep_set_novalidate_class?
> > (perhaps with rename). What do you think about the patch below?
> > 
> > With this patch __lockdep_no_validate__ means "automatically nested",
> 
> Yes, I suppose that might work, it would allow some validation.

I haven't seen the patch, but I'm not so sure it will work.  Suppose we
have two devices, D1 and D2, and some other mutex, M.  Then the locking
pattern:

	lock(D1);
	lock(M);
	unlock(M);
	unlock(D1);

generally should not conflict with:

	lock(M);
	lock(D2);
	unlock(D2);
	unlock(M);

even though D1's and D2's locks belong to the same class.  For example,
M might be a mutex embedded in the private data associated with D1, and
D2 might be a child of D1.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/