From: Sasha Levin <sasha.levin@oracle.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: torvalds@linux-foundation.org, mingo@kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 7/9] liblockdep: Support using LD_PRELOAD
Date: Fri, 10 May 2013 12:42:21 -0400 [thread overview]
Message-ID: <518D236D.6000608@oracle.com> (raw)
In-Reply-To: <20130510162115.GB885@dyad.programming.kicks-ass.net>
On 05/10/2013 12:21 PM, Peter Zijlstra wrote:
> On Fri, May 10, 2013 at 12:06:46PM -0400, Sasha Levin wrote:
>> On 05/10/2013 09:57 AM, Peter Zijlstra wrote:
>>> So you're doing instance tracking and not creating classes like the kernel
>>> lockdep does? While that reduces false positives it also greatly reduces the
>>> effectiveness of lockdep.
>>>
>>> The power of lock-classes is that it increases the chance of catching potential
>>> deadlocks without there ever actually being a deadlock.
>>
>> Originally I had classes working as you've pointed out, until the first time I've
>> tried running lockdep on qemu.
>>
>> They appear to have wrappers for every api call known to man, including all the
>> posix locking apis.
>>
>> Basically, instead of directly calling pthread_mutex_lock() for example, there's
>> a wrapper named qemu_mutex_lock() that calls the api above:
>>
>> void qemu_mutex_lock(QemuMutex *mutex)
>> {
>> int err;
>>
>> err = pthread_mutex_lock(&mutex->lock);
>> if (err)
>> error_exit(err, __func__);
>> }
>>
>> So as you might imagine, the first time I ran it my log exploded with warnings.
>>
>> I've poked around the source of other big projects, and the example above is
>> somewhat common with projects that wrap everything to be compatible with different
>> architectures or apis - which is something that doesn't happen in the kernel.
>
> Urgh.. yes that might be a problem. Still it is something that should at least
> be clearly stated somewhere (the Changelog for one).
>
> Not being able to do classes sucks though :/
>
> Hmm, we could do something like:
>
> $ LIBLOCKDEP_CLASS_DEPTH=n LD_PRELOAD=liblockdep.so my_app
>
> where an @n of -1 would indicate per-instance classes and 0+ would be the
> __builtin_return_address(n). That way, the above qemu thing should work with 1;
> which should be the return address of the wrapper.
>
> Of course, projects mixing different wrapper depths will be immense 'fun' :/
Like, um, qemu? :)
You think that the only pthread_mutex_lock() calls you'd see are in the wrapper
module, right? grepping for pthread_mutex_lock will show that that's not the case...
Which is also why I'd rather leave this part out for now. I really want something
pretty simple that works for *most* cases and relies *mostly* on kernel/lockdep.c
logic instead of custom liblockdep stuff (right now the only "logic" in liblockdep
is rb-tree loockups) so that we won't scare Linus.
Also, if qemu (for example) really wants to get proper lockdep support for their
testing all it takes is just an extra call to liblockdep's mutex init api inside
their code - it means that they would need to modify their code and actually link
with liblockdep, but there's a limit to the magic that can be done from outside the code.
(I'm using qemu as an example not because I think it's horrible or something like
that, but because it's a great example for a mature big project that is still
relatively easy to work with).
Thanks,
Sasha
next prev parent reply other threads:[~2013-05-10 16:42 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-09 15:58 [PATCH v3 0/9] liblockdep: userspace lockdep Sasha Levin
2013-05-09 15:58 ` [PATCH v3 1/9] lockdep: Be nice about building from userspace Sasha Levin
2013-05-09 15:58 ` [PATCH v3 2/9] liblockdep: Wrap kernel/lockdep.c to allow usage " Sasha Levin
2013-05-10 9:18 ` Peter Zijlstra
2013-05-10 9:38 ` Ingo Molnar
2013-05-10 10:11 ` Peter Zijlstra
2013-05-10 13:23 ` Sasha Levin
2013-05-10 14:03 ` Peter Zijlstra
2013-05-10 14:11 ` Sasha Levin
2013-05-10 14:12 ` Peter Zijlstra
2013-05-10 14:19 ` Peter Zijlstra
2013-05-10 14:23 ` Sasha Levin
2013-05-10 13:58 ` Peter Zijlstra
2013-05-09 15:58 ` [PATCH v3 3/9] liblockdep: Add public headers for pthread_mutex_t implementation Sasha Levin
2013-05-09 15:58 ` [PATCH v3 4/9] liblockdep: Add pthread_mutex_t test suite Sasha Levin
2013-05-09 15:58 ` [PATCH v3 5/9] liblockdep: Add public headers for pthread_rwlock_t implementation Sasha Levin
2013-05-09 15:58 ` [PATCH v3 6/9] liblockdep: Add pthread_rwlock_t test suite Sasha Levin
2013-05-09 15:58 ` [PATCH v3 7/9] liblockdep: Support using LD_PRELOAD Sasha Levin
2013-05-10 11:17 ` Peter Zijlstra
2013-05-10 11:23 ` Ingo Molnar
2013-05-10 11:46 ` Peter Zijlstra
2013-05-10 13:11 ` Peter Zijlstra
2013-05-10 13:57 ` Peter Zijlstra
2013-05-10 16:06 ` Sasha Levin
2013-05-10 16:21 ` Peter Zijlstra
2013-05-10 16:42 ` Sasha Levin [this message]
2013-05-09 15:58 ` [PATCH v3 8/9] liblockdep: Add the 'lockdep' user-space utility Sasha Levin
2013-05-10 9:19 ` Peter Zijlstra
2013-05-10 9:35 ` Ingo Molnar
2013-05-10 10:41 ` Peter Zijlstra
2013-05-10 11:00 ` Ingo Molnar
2013-05-10 11:10 ` Borislav Petkov
2013-05-10 11:07 ` Ingo Molnar
2013-05-09 15:58 ` [PATCH v3 9/9] liblockdep: Add a MAINTAINERS entry Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=518D236D.6000608@oracle.com \
--to=sasha.levin@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox