linux-toradex.git/drivers/oprofile/buffer_sync.c, branch v2.6.32.64

oprofile: Fix locking dependency in sync_start()

2011-12-21T21:04:51+00:00

commit 130c5ce716c9bfd1c2a2ec840a746eb7ff9ce1e6 upstream.

This fixes the A->B/B->A locking dependency, see the warning below.

The function task_exit_notify() is called with (task_exit_notifier)
.rwsem set and then calls sync_buffer() which locks buffer_mutex. In
sync_start() the buffer_mutex was set to prevent notifier functions to
be started before sync_start() is finished. But when registering the
notifier, (task_exit_notifier).rwsem is locked too, but now in
different order than in sync_buffer(). In theory this causes a locking
dependency, what does not occur in practice since task_exit_notify()
is always called after the notifier is registered which means the lock
is already released.

However, after checking the notifier functions it turned out the
buffer_mutex in sync_start() is unnecessary. This is because
sync_buffer() may be called from the notifiers even if sync_start()
did not finish yet, the buffers are already allocated but empty. No
need to protect this with the mutex.

So we fix this theoretical locking dependency by removing buffer_mutex
in sync_start(). This is similar to the implementation before commit:

 750d857 oprofile: fix crash when accessing freed task structs

which introduced the locking dependency.

Lockdep warning:

oprofiled/4447 is trying to acquire lock:
 (buffer_mutex){+.+...}, at: [] sync_buffer+0x31/0x3ec [oprofile]

but task is already holding lock:
 ((task_exit_notifier).rwsem){++++..}, at: [] __blocking_notifier_call_chain+0x39/0x67

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 ((task_exit_notifier).rwsem){++++..}:
       [] lock_acquire+0xf8/0x11e
       [] down_write+0x44/0x67
       [] blocking_notifier_chain_register+0x52/0x8b
       [] profile_event_register+0x2d/0x2f
       [] sync_start+0x47/0xc6 [oprofile]
       [] oprofile_setup+0x60/0xa5 [oprofile]
       [] event_buffer_open+0x59/0x8c [oprofile]
       [] __dentry_open+0x1eb/0x308
       [] nameidata_to_filp+0x60/0x67
       [] do_last+0x5be/0x6b2
       [] path_openat+0xc7/0x360
       [] do_filp_open+0x3d/0x8c
       [] do_sys_open+0x110/0x1a9
       [] sys_open+0x20/0x22
       [] system_call_fastpath+0x16/0x1b

-> #0 (buffer_mutex){+.+...}:
       [] __lock_acquire+0x1085/0x1711
       [] lock_acquire+0xf8/0x11e
       [] mutex_lock_nested+0x63/0x309
       [] sync_buffer+0x31/0x3ec [oprofile]
       [] task_exit_notify+0x16/0x1a [oprofile]
       [] notifier_call_chain+0x37/0x63
       [] __blocking_notifier_call_chain+0x50/0x67
       [] blocking_notifier_call_chain+0x14/0x16
       [] profile_task_exit+0x1a/0x1c
       [] do_exit+0x2a/0x6fc
       [] do_group_exit+0x83/0xae
       [] sys_exit_group+0x17/0x1b
       [] system_call_fastpath+0x16/0x1b

other info that might help us debug this:

1 lock held by oprofiled/4447:
 #0:  ((task_exit_notifier).rwsem){++++..}, at: [] __blocking_notifier_call_chain+0x39/0x67

stack backtrace:
Pid: 4447, comm: oprofiled Not tainted 2.6.39-00007-gcf4d8d4 #10
Call Trace:
 [] print_circular_bug+0xae/0xbc
 [] __lock_acquire+0x1085/0x1711
 [] ? sync_buffer+0x31/0x3ec [oprofile]
 [] lock_acquire+0xf8/0x11e
 [] ? sync_buffer+0x31/0x3ec [oprofile]
 [] ? mark_lock+0x42f/0x552
 [] ? sync_buffer+0x31/0x3ec [oprofile]
 [] mutex_lock_nested+0x63/0x309
 [] ? sync_buffer+0x31/0x3ec [oprofile]
 [] sync_buffer+0x31/0x3ec [oprofile]
 [] ? __blocking_notifier_call_chain+0x39/0x67
 [] ? __blocking_notifier_call_chain+0x39/0x67
 [] task_exit_notify+0x16/0x1a [oprofile]
 [] notifier_call_chain+0x37/0x63
 [] __blocking_notifier_call_chain+0x50/0x67
 [] blocking_notifier_call_chain+0x14/0x16
 [] profile_task_exit+0x1a/0x1c
 [] do_exit+0x2a/0x6fc
 [] ? retint_swapgs+0xe/0x13
 [] do_group_exit+0x83/0xae
 [] sys_exit_group+0x17/0x1b
 [] system_call_fastpath+0x16/0x1b

Reported-by: Marcin Slusarz 
Cc: Carl Love 
Signed-off-by: Robert Richter 
Signed-off-by: Greg Kroah-Hartman

oprofile: Free potentially owned tasks in case of errors

2011-12-21T21:04:51+00:00

commit 6ac6519b93065625119a347be1cbcc1b89edb773 upstream.

After registering the task free notifier we possibly have tasks in our
dying_tasks list. Free them after unregistering the notifier in case
of an error.

Signed-off-by: Robert Richter 
Signed-off-by: Greg Kroah-Hartman

oprofile: fix crash when accessing freed task structs

2010-09-20T20:17:50+00:00

commit 750d857c682f4db60d14722d430c7ccc35070962 upstream.

This patch fixes a crash during shutdown reported below. The crash is
caused by accessing already freed task structs. The fix changes the
order for registering and unregistering notifier callbacks.

All notifiers must be initialized before buffers start working. To
stop buffer synchronization we cancel all workqueues, unregister the
notifier callback and then flush all buffers. After all of this we
finally can free all tasks listed.

This should avoid accessing freed tasks.

On 22.07.10 01:14:40, Benjamin Herrenschmidt wrote:

> So the initial observation is a spinlock bad magic followed by a crash
> in the spinlock debug code:
>
> [ 1541.586531] BUG: spinlock bad magic on CPU#5, events/5/136
> [ 1541.597564] Unable to handle kernel paging request for data at address 0x6b6b6b6b6b6b6d03
>
> Backtrace looks like:
>
>       spin_bug+0x74/0xd4
>       ._raw_spin_lock+0x48/0x184
>       ._spin_lock+0x10/0x24
>       .get_task_mm+0x28/0x8c
>       .sync_buffer+0x1b4/0x598
>       .wq_sync_buffer+0xa0/0xdc
>       .worker_thread+0x1d8/0x2a8
>       .kthread+0xa8/0xb4
>       .kernel_thread+0x54/0x70
>
> So we are accessing a freed task struct in the work queue when
> processing the samples.

Reported-by: Benjamin Herrenschmidt 
Signed-off-by: Robert Richter 
Signed-off-by: Greg Kroah-Hartman

cpumask: use zalloc_cpumask_var() where possible

2009-09-24T00:04:24+00:00

Remove open-coded zalloc_cpumask_var() and zalloc_cpumask_var_node().

Signed-off-by: Li Zefan 
Signed-off-by: Rusty Russell

cpumask: modifiy oprofile initialization

2009-01-22T13:37:06+00:00

Delta patch to f7df8ed164996cd2c6aca9674388be6ef78d8b37 for
tip/cpus4096.

Moved initialization to sync_start()/sync_stop(). No changes needed in
buffer_sync.h and oprof.c anymore.

Signed-off-by: Robert Richter 
Signed-off-by: Ingo Molnar

cpumask: convert misc driver functions

2009-01-11T18:12:52+00:00

Impact: use new cpumask API.

Convert misc driver functions to use struct cpumask.

To Do:
  - Convert iucv_buffer_cpumask to cpumask_var_t.

Signed-off-by: Rusty Russell 
Signed-off-by: Mike Travis 
Acked-by: Dean Nelson 
Cc: Robert Richter 
Cc: oprofile-list@lists.sf.net
Cc: Jeremy Fitzhardinge 
Cc: Chris Wright 
Cc: virtualization@lists.osdl.org
Cc: xen-devel@lists.xensource.com
Cc: Ursula Braun 
Cc: linux390@de.ibm.com
Cc: linux-s390@vger.kernel.org

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile

2009-01-09T20:43:06+00:00

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile: (31 commits)
  powerpc/oprofile: fix whitespaces in op_model_cell.c
  powerpc/oprofile: IBM CELL: add SPU event profiling support
  powerpc/oprofile: fix cell/pr_util.h
  powerpc/oprofile: IBM CELL: cleanup and restructuring
  oprofile: make new cpu buffer functions part of the api
  oprofile: remove #ifdef CONFIG_OPROFILE_IBS in non-ibs code
  ring_buffer: fix ring_buffer_event_length()
  oprofile: use new data sample format for ibs
  oprofile: add op_cpu_buffer_get_data()
  oprofile: add op_cpu_buffer_add_data()
  oprofile: rework implementation of cpu buffer events
  oprofile: modify op_cpu_buffer_read_entry()
  oprofile: add op_cpu_buffer_write_reserve()
  oprofile: rename variables in add_ibs_begin()
  oprofile: rename add_sample() in cpu_buffer.c
  oprofile: rename variable ibs_allowed to has_ibs in op_model_amd.c
  oprofile: making add_sample_entry() inline
  oprofile: remove backtrace code for ibs
  oprofile: remove unused ibs macro
  oprofile: remove unused components in struct oprofile_cpu_buffer
  ...

oprofile: remove #ifdef CONFIG_OPROFILE_IBS in non-ibs code

2009-01-07T21:47:56+00:00

The ifdefs can be removed since the code is no longer ibs specific and
can be used for other purposes as well. IBS specific code is only in
op_model_amd.c.

Signed-off-by: Robert Richter

oprofile: use new data sample format for ibs

2009-01-07T21:47:23+00:00

The new ring buffer implementation allows the storage of samples with
different size. This patch implements the usage of the new sample
format to store ibs samples in the cpu buffer. Until now, writing to
the cpu buffer could lead to incomplete sampling sequences since IBS
samples were transfered in multiple samples. Due to a full buffer,
data could be lost at any time. This can't happen any more since the
complete data is reserved in advance and then stored in a single
sample.

Signed-off-by: Robert Richter

oprofile: add op_cpu_buffer_get_data()

2009-01-07T21:45:46+00:00

This function provides access to attached data of a sample. It returns
the size of data including the current value. Also,
op_cpu_buffer_get_size() is available to check if there is data
attached.

Signed-off-by: Robert Richter