09 Oct, 2012

1 commit

  • Some security modules and oprofile still uses VM_EXECUTABLE for retrieving
    a task's executable file. After this patch they will use mm->exe_file
    directly. mm->exe_file is protected with mm->mmap_sem, so locking stays
    the same.

    Signed-off-by: Konstantin Khlebnikov
    Acked-by: Chris Metcalf [arch/tile]
    Acked-by: Tetsuo Handa [tomoyo]
    Cc: Alexander Viro
    Cc: Carsten Otte
    Cc: Cyrill Gorcunov
    Cc: Eric Paris
    Cc: H. Peter Anvin
    Cc: Hugh Dickins
    Cc: Ingo Molnar
    Acked-by: James Morris
    Cc: Jason Baron
    Cc: Kentaro Takeda
    Cc: Matt Helsley
    Cc: Nick Piggin
    Cc: Oleg Nesterov
    Cc: Peter Zijlstra
    Cc: Robert Richter
    Cc: Suresh Siddha
    Cc: Venkatesh Pallipadi
    Acked-by: Linus Torvalds
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Konstantin Khlebnikov
     

31 May, 2011

2 commits

  • 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
    Cc: # .36+
    Signed-off-by: Robert Richter

    Robert Richter
     
  • 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.

    Cc: # .36+
    Signed-off-by: Robert Richter

    Robert Richter
     

29 Oct, 2010

1 commit

  • flush_scheduled_work() is deprecated and scheduled to be removed.
    sync_stop() currently cancels cpu_buffer works inside buffer_mutex and
    flushes the system workqueue outside. Instead, split end_cpu_work()
    into two parts - stopping further work enqueues and flushing works -
    and do the former inside buffer_mutex and latter outside.

    For stable kernels v2.6.35.y and v2.6.36.y.

    Signed-off-by: Tejun Heo
    Cc: stable@kernel.org
    Signed-off-by: Robert Richter

    Tejun Heo
     

25 Aug, 2010

1 commit

  • 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
    Cc: stable@kernel.org
    Signed-off-by: Robert Richter

    Robert Richter
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

24 Sep, 2009

1 commit


22 Jan, 2009

1 commit


12 Jan, 2009

1 commit

  • 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

    Rusty Russell
     

10 Jan, 2009

1 commit

  • * '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
    ...

    Linus Torvalds
     

08 Jan, 2009

9 commits


01 Jan, 2009

1 commit

  • struct dentry is one of the most critical structures in the kernel. So it's
    sad to see it going neglected.

    With CONFIG_PROFILING turned on (which is probably the common case at least
    for distros and kernel developers), sizeof(struct dcache) == 208 here
    (64-bit). This gives 19 objects per slab.

    I packed d_mounted into a hole, and took another 4 bytes off the inline
    name length to take the padding out from the end of the structure. This
    shinks it to 200 bytes. I could have gone the other way and increased the
    length to 40, but I'm aiming for a magic number, read on...

    I then got rid of the d_cookie pointer. This shrinks it to 192 bytes. Rant:
    why was this ever a good idea? The cookie system should increase its hash
    size or use a tree or something if lookups are a problem. Also the "fast
    dcookie lookups" in oprofile should be moved into the dcookie code -- how
    can oprofile possibly care about the dcookie_mutex? It gets dropped after
    get_dcookie() returns so it can't be providing any sort of protection.

    At 192 bytes, 21 objects fit into a 4K page, saving about 3MB on my system
    with ~140 000 entries allocated. 192 is also a multiple of 64, so we get
    nice cacheline alignment on 64 and 32 byte line systems -- any given dentry
    will now require 3 cachelines to touch all fields wheras previously it
    would require 4.

    I know the inline name size was chosen quite carefully, however with the
    reduction in cacheline footprint, it should actually be just about as fast
    to do a name lookup for a 36 character name as it was before the patch (and
    faster for other sizes). The memory footprint savings for names which are
    36 bytes long should more than make up for the memory cost for
    33-36 byte names.

    Performance is a feature...

    Signed-off-by: Al Viro

    Nick Piggin
     

30 Dec, 2008

2 commits


29 Dec, 2008

1 commit


10 Dec, 2008

6 commits

  • This patch replaces the current oprofile cpu buffer implementation
    with the ring buffer provided by the tracing framework. The motivation
    here is to leave the pain of implementing ring buffers to others. Oh,
    no, there are more advantages. Main reason is the support of different
    sample sizes that could be stored in the buffer. Use cases for this
    are IBS and Cell spu profiling. Using the new ring buffer ensures
    valid and complete samples and allows copying the cpu buffer stateless
    without knowing its content. Second it will use generic kernel API and
    also reduce code size. And hopefully, there are less bugs.

    Since the new tracing ring buffer implementation uses spin locks to
    protect the buffer during read/write access, it is difficult to use
    the buffer in an NMI handler. In this case, writing to the buffer by
    the NMI handler (x86) could occur also during critical sections when
    reading the buffer. To avoid this, there are 2 buffers for independent
    read and write access. Read access is in process context only, write
    access only in the NMI handler. If the read buffer runs empty, both
    buffers are swapped atomically. There is potentially a small window
    during swapping where the buffers are disabled and samples could be
    lost.

    Using 2 buffers is a little bit overhead, but the solution is clear
    and does not require changes in the ring buffer implementation. It can
    be changed to a single buffer solution when the ring buffer access is
    implemented as non-locking atomic code.

    The new buffer requires more size to store the same amount of samples
    because each sample includes an u32 header. Also, there is more code
    to execute for buffer access. Nonetheless, the buffer implementation
    is proven in the ftrace environment and worth to use also in oprofile.

    Patches that changes the internal IBS buffer usage will follow.

    Cc: Steven Rostedt
    Signed-off-by: Robert Richter

    Robert Richter
     
  • This is in preparation for changes in the cpu buffer implementation.

    Signed-off-by: Robert Richter

    Robert Richter
     
  • This is in preparation for changes in the cpu buffer implementation.

    Signed-off-by: Robert Richter

    Robert Richter
     
  • This is in preparation for changes in the cpu buffer implementation.

    Signed-off-by: Robert Richter

    Robert Richter
     
  • Signed-off-by: Robert Richter

    Robert Richter
     
  • Signed-off-by: Robert Richter

    Robert Richter
     

24 Oct, 2008

1 commit

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile: (21 commits)
    OProfile: Fix buffer synchronization for IBS
    oprofile: hotplug cpu fix
    oprofile: fixing whitespaces in arch/x86/oprofile/*
    oprofile: fixing whitespaces in arch/x86/oprofile/*
    oprofile: fixing whitespaces in drivers/oprofile/*
    x86/oprofile: add the logic for enabling additional IBS bits
    x86/oprofile: reordering functions in nmi_int.c
    x86/oprofile: removing unused function parameter in add_ibs_begin()
    oprofile: more whitespace fixes
    oprofile: whitespace fixes
    OProfile: Rename IBS sysfs dir into "ibs_op"
    OProfile: Rework string handling in setup_ibs_files()
    OProfile: Rework oprofile_add_ibs_sample() function
    oprofile: discover counters for op ppro too
    oprofile: Implement Intel architectural perfmon support
    oprofile: Don't report Nehalem as core_2
    oprofile: drop const in num counters field
    Revert "Oprofile Multiplexing Patch"
    x86, oprofile: BUG: using smp_processor_id() in preemptible code
    x86/oprofile: fix on_each_cpu build error
    ...

    Manually fixed trivial conflicts in
    drivers/oprofile/{cpu_buffer.c,event_buffer.h}

    Linus Torvalds
     

21 Oct, 2008

1 commit

  • The issue is the SPU code is not holding the kernel mutex lock while
    adding samples to the kernel buffer.

    This patch creates per SPU buffers to hold the data. Data
    is added to the buffers from in interrupt context. The data
    is periodically pushed to the kernel buffer via a new Oprofile
    function oprofile_put_buff(). The oprofile_put_buff() function
    is called via a work queue enabling the funtion to acquire the
    mutex lock.

    The existing user controls for adjusting the per CPU buffer
    size is used to control the size of the per SPU buffers.
    Similarly, overflows of the SPU buffers are reported by
    incrementing the per CPU buffer stats. This eliminates the
    need to have architecture specific controls for the per SPU
    buffers which is not acceptable to the OProfile user tool
    maintainer.

    The export of the oprofile add_event_entry() is removed as it
    is no longer needed given this patch.

    Note, this patch has not addressed the issue of indexing arrays
    by the spu number. This still needs to be fixed as the spu
    numbering is not guarenteed to be 0 to max_num_spus-1.

    Signed-off-by: Carl Love
    Signed-off-by: Maynard Johnson
    Signed-off-by: Arnd Bergmann
    Acked-by: Acked-by: Robert Richter
    Signed-off-by: Benjamin Herrenschmidt

    Carl Love
     

20 Oct, 2008

1 commit

  • The patch is needed since there is some IBS code in add_ibs_begin()
    that handles more than one sample per iteration. This requires calling
    get_slots() during each loop.

    This fixes the current problem, but a proper solution that reworks the
    cpu buffer synchronization is needed here in the future.

    Signed-off-by: Barry Kasindorf
    Signed-off-by: Robert Richter

    Barry Kasindorf
     

16 Oct, 2008

2 commits


26 Jul, 2008

4 commits


28 Apr, 2008

1 commit


15 Feb, 2008

1 commit

  • get_dcookie() is always called with a dentry and a vfsmount from a struct
    path. Make get_dcookie() take it directly as an argument.

    [akpm@linux-foundation.org: coding-style fixes]
    Signed-off-by: Jan Blunck
    Acked-by: Christoph Hellwig
    Cc: Al Viro
    Cc: "J. Bruce Fields"
    Cc: Neil Brown
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck