linux-kernel - Re: [PATCH 7/7] driver-core : convert semaphore to mutex in struct class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080117083810.GA1710@ff.dom.local>
Date:	Thu, 17 Jan 2008 09:38:10 +0100
From:	Jarek Poplawski <jarkao2@...il.com>
To:	Dave Young <hidave.darkstar@...il.com>
Cc:	Greg KH <gregkh@...e.de>, stefanr@...6.in-berlin.de,
	david-b@...bell.net, stern@...land.harvard.edu,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/7] driver-core : convert semaphore to mutex in struct
	class

On 17-01-2008 02:17, Dave Young wrote:
> On Jan 16, 2008 4:34 PM, Jarek Poplawski <jarkao2@...il.com> wrote:
>> On Wed, Jan 16, 2008 at 09:03:03AM +0800, Dave Young wrote:
>> ...
>>> The lockdep warining was posted in the below thread, actually, I have
>>> built and run this patced kernel for several days, there's no more
>>> warnings.
>>> http://lkml.org/lkml/2008/1/3/2
>> Right... But, with something like this:
>>
>> ... have_some_fun(... cls)
>> {
>>         mutex_lock_nested(&cls->mutex, SINGLE_DEPTH_NESTING);
>>         have_other_fun(cls);
>>         mutex_unlock(&cls->mutex);
>>
>> }
>>
>> ... have_more_fun(...)
>> {
>>         ...
>>
>>         mutex_init(&cls->mutex);
>>
>>         mutex_lock(&cls->mutex);
>>         have_some_fun(cls);
>>         mutex_unlock(&cls->mutex);
>> }
>>
>> probably you wouldn't get any lockdep warning too...
> 
> Sorry for late reply.
> Actually, I don't know much about lockdep. Could you tell how to use
> it properly in this scenario?

As you have noticed while working on this patch, if two different
instances of the same structure containig some lock are created in
the same place, lockdep will treat this one (the same) lock. It looks
strange, but actually it's a feature which enables to track
dependencies between different locks on 'class' level instead of
instance 'level'. The downside is that lockdep is very often too
sensitive by default, so you have to 'annotate' when instancess are
actually on different level (e.g. parents and children here) and
could be locked at the same time or in some order.

You can use e.g. mutex_lock_nested() or lockdep_set_class*() for this.
Then lockdep simply trusts you, and starts to think they are different
locks. If you do it wrong there will be simply no more warnings, but
undercover lockups still possible (and diagnosing a bit harder then).
So, since in your patch there are two levels of locking, and you
started to annotate lockdep about a child taking parent's class lock
with: 
mutex_lock_nested(&parent_class->mutex, SINGLE_DEPTH_NESTING);

you should do the same everywhere in a situation like this.
lockdep will treat this simply as lock B vs. A (mutex_lock(&cls))
dependencies.

Regards,
Jarek P.

PS: BTW, it seems after this patch 1/1 the locking was changed a bit,
so these previous tests could be not enough.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/