Re: [Cbe-oss-dev] [PATCH] Cell OProfile: SPU mutex lock fix

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Arnd Bergmann <arnd@...>
Cc: <cbe-oss-dev@...>, <linuxppc-dev@...>, <linux-kernel@...>, cel <cel@...>
Date: Wednesday, April 2, 2008 - 12:42 pm

On Wed, 2008-04-02 at 07:21 +0200, Arnd Bergmann wrote:

The generic OProfile code defines a mutex lock, called buffer_mutex, to
protect the kernel/daemon data buffer from being writen by the kernal
and simultaneously read by the Daemon.  When adding a PPU sample the
oprofile routine  oprofile_add_ext_sample(pc, regs, i, is_kernel) is
called from the interrupt context to request the sample be stored.  The
generic oprofile code takes care of passing the data to a non interrupt
context where the mutex lock is held and the necessary sequence of data
is written into the kernel/daemon data buffer.  However, OProfile does
not have any built in functions for handling the SPU.  Hence, we have to
implement the code to capture the data in the interrupt context, pass it
to a non interrupt context and put it into the buffer.  This was not
done correctly in the original implementation.  Specifically, the mutex
lock was not being held.  

Writing data to the OProfile buffer consists of a sequence of items.
For example when writing an SPU entry, first comes the escape code so
the daemon knows this is a new entry.  The next item is the SPU context
switch code which says the data which will follow is the information
about a new context.  There is a different code to identify the data as
an address sample.  Finally the data about the SPU context switch is
entered into the buffer.  The issue is the OProfile daemon is read all
of the entire sequence of items then process the data.  Without the
mutex lock, the daemon may read part of the sequence try to process it
before everything is written into the buffer.  When the daemon reads
again, it doesn't see the escape code as the first item and isn't smart
enough to realize it is part of a previous sequence.  The generic
OProfile code defines the mutex lock and calls it buffer_mutex.  The
OProfile kernel/daemon API uses the mutex lock. The mutex lock can only
be held in a non interrupt context.  The current implementation uses a
spin lock to make sure the kernel writes each sequence if items into the
buffer but since the API does not use a spin lock we have no way to
prevent the daemon from reading the buffer until the entire sequence of
items has been written to the buffer.  Hence the need to hold the
buffer_mutex lock which prevents the daemon from accessing the buffer.


The current implementation uses the hrtimer to schedule when to read the
trace buffer the next time.  This patch does not change how the
scheduling of the buffer reads is done.  Yes, you could change the
implementation to use workqueues instead.  If you feel that it is better
to use the workqueue then we could make that change.  Not sure that
making that change in this bug fix patch is appropriate.  I would need
to create a second patch for that change.

As stated earlier, the generic OProfile code defines the variable
"buffer_mutex".  Changing the name in the generic OProfile code is
beyond the scope of this patch.


This is the data same data that was being put into the event buffer
directly from the interrupt context.  We need to store the data that is
only available in the interrupt context so the same data can be put into
the buffer by the work queue function in the non interrupt context.
This is the declaration of the data needed per SPU.  Below in the
spu_cntx_sw_data structure, we declare an array of entries so we can
store the switch

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [Cbe-oss-dev] [PATCH] Cell OProfile: SPU mutex lock fix, Carl Love, (Wed Apr 2, 12:42 pm)