Re: oprofile: possible circular locking dependency detected

[Robert R. <rob...@am...>]

On 18.01.11 17:16:40, Carl Love wrote:
> Marcin Slusarz <mar...@gm...> wrote on 01/15/2011 03:13:15 PM:
> > Lockdep finds possible circular locking dependency during
> > opcontrol --start on 2.6.37 kernel:

> We ran into this last week or so when we were trying to find an issue with perf
> and OProfile.
> It comes about because you have the config options LOCKDEP_SUPPORT (I am fairly
> sure that is
> the config option) enabled.  We spent some time looking into it and decided
> that since
> sync_start() must complete before the sync_buffer() routine can be called that
> you couldn't
> get a deadlock between these routines.  However, the same lock dependencies
> exist in sync_stop()
> and sync_buffer().  In this case, the sync_buffer() routine has been enabled
> and could be
> running when sync_exit() stops.  At least, we couldn't see any reason why that
> would not be
> the case.  We never got to proving that it actually ever happens.
> 
> From searching through the changes, it appears that last fall, I believe it was
> the Sept 2010
> time frame Robert Richter added the mutex to sync_start() and sync_stop() as
> part of
> fixing another issue.  If I recall correctly, the issue was trying to process
> samples for a
> process after the task struct for the process was gone.  The mutexes were added
> as well as
> moving some code around to correct the issue.  
> 
> We looked at the code with the mutexes and don't think the are needed.  I was
> planning on posting
> a message to the list asking about this change but hadn't gotten to it when
> this message came out.  
> I guess what needs to be done is to evaluate if we really need the mutex in the
> sync_start()
> and sync_stop() functions.

I just sent a fix for this to the lkml:

 [PATCH 2/3] oprofile: Fix locking dependency in sync_start()

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center