lkml.org 
[lkml]   [2010]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] perf lock: Fix state machine to recognize lock sequence
On 04/21/10 10:26, Frederic Weisbecker wrote:
> On Fri, Apr 16, 2010 at 05:44:06PM +0900, Hitoshi Mitake wrote:
>> Hi Ingo,
>>
>> I'm developing the model to recognize the correct sequence of lock
events.
>> Previous state machine of perf lock was really broken.
>> This patch improves it a little.
>>
>> This patch prepares the array of state machine represents lock
sequence for each threads.
>> These state machines represent one of these sequence:
>>
>> 1) acquire -> acquired -> release
>> 2) acquire -> contended -> acquired -> release
>> 3) acquire (w/ try) -> release
>> 4) acquire (w/ read) -> release
>>
>> The case of 4) is a little special.
>> Double acquire of read lock is allowed, so state machine of sequence
>> counts read lock number, and permit double acquire and release.
>>
>> But, things are not so simple. Something of my model is still wrong.
>> I counted the number of lock instances with bad sequence,
>> and ratio is like this (case of tracing whoami): bad:122, total:1956
>
>
>
> I just gave your patch a try and it's worse: almost every sequences
> were reported bad (it wasn't working either before your patch :)
>
> This is not the fault of your patch though. Actually your patch seems to
> be a nice improvement.

Thanks for your review, Frederic!

>
> In fact I just found two things:
>
> 1) We are working on tasks in pid basis. We should work on a task by
using
> its tid.
> In fact we are processing the sequences of several threads in a
process as
> if we were dealing with a single task.
>
> If A and B are two threads belonging to a same process, and if we have:
>
> A: acquire lock 1, release lock 1
> B: acquire lock 2, release lock 2
>
> ...then we are dealing with a random mess of sequences:
>
> AB: acquire lock 1, acquire lock 2, release lock 1, and any kind of
random
> things like this.


Ah, I missed tid. I'll fix this point.

>
> 2) I can't get lock_acquired traces. Not sure why yet...

Really? It's mystery... I'll seek the cause.

>
>
>>
>> There is another new bad thing.
>> The size of array of state machine is equal to max depth lockdep
defines.
>> If perf lock record tries to record lock events of the programs with
lots of
>> system call like "perf bench sched messaging", the array will be
exhausted :(
>
>
>
> Yeah, I suggest you use a list for that in fact. The max lockdep
depth may
> change in the future, or become variable, so we can't relay on that.

Yeah, I'll use list or hashtable.

>
> But that's still a cool improvement.
>
> I'm queuing this patch.

Thanks! But I have to fix some points based on your advice.
Should I send v2 patch or make fix on your tree?

Thanks,
Hitoshi


\
 
 \ /
  Last update: 2010-04-21 11:15    [W:0.132 / U:3.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site