All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Ming Lei <tom.leiming@gmail.com>
Cc: Christoph Lameter <cl@linux.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op
Date: Tue, 21 Feb 2012 09:30:42 -0800	[thread overview]
Message-ID: <20120221173042.GC12236@google.com> (raw)
In-Reply-To: <1329296078-17751-1-git-send-email-tom.leiming@gmail.com>

On Wed, Feb 15, 2012 at 04:54:38PM +0800, Ming Lei wrote:
> It doesn't make sense to trace irq off or do irq flags
> lock proving inside 'this_cpu' operations, so replace local_irq_*
> with raw_local_irq_* in 'this_cpu' op.
> 
> Also the patch fixes onelockdep warning[1] by the replacement, see
> below:
> 
> In commit: 933393f58fef9963eac61db8093689544e29a600(percpu:
> Remove irqsafe_cpu_xxx variants), local_irq_save/restore(flags) are
> added inside this_cpu_inc operation, so that trace_hardirqs_off_caller
> will be called by trace_hardirqs_on_caller directly because
> __debug_atomic_inc is implemented as this_cpu_inc, which may trigger
> the lockdep warning[1], for example in the below ARM scenary:
> 
> 	kernel_thread_helper	/*irq disabled*/
> 		->trace_hardirqs_on_caller	/*hardirqs_enabled was set*/
> 			->trace_hardirqs_off_caller	/*hardirqs_enabled cleared*/
> 				__this_cpu_add(redundant_hardirqs_on)
> 			->trace_hardirqs_off_caller	/*irq disabled, so call here*/
> 
> The 'unannotated irqs-on' warning will be triggered somewhere because
> irq is just enabled after the irq trace in kernel_thread_helper.
> 
> [1],
> [    0.162841] ------------[ cut here ]------------
> [    0.167694] WARNING: at kernel/lockdep.c:3493 check_flags+0xc0/0x1d0()
> [    0.174468] Modules linked in:
> [    0.177703] Backtrace:
> [    0.180328] [<c00171f0>] (dump_backtrace+0x0/0x110) from [<c0412320>] (dump_stack+0x18/0x1c)
> [    0.189086]  r6:c051f778 r5:00000da5 r4:00000000 r3:60000093
> [    0.195007] [<c0412308>] (dump_stack+0x0/0x1c) from [<c00410e8>] (warn_slowpath_common+0x54/0x6c)
> [    0.204223] [<c0041094>] (warn_slowpath_common+0x0/0x6c) from [<c0041124>] (warn_slowpath_null+0x24/0x2c)
> [    0.214111]  r8:00000000 r7:00000000 r6:ee069598 r5:60000013 r4:ee082000
> [    0.220825] r3:00000009
> [    0.223693] [<c0041100>] (warn_slowpath_null+0x0/0x2c) from [<c0088f38>] (check_flags+0xc0/0x1d0)
> [    0.232910] [<c0088e78>] (check_flags+0x0/0x1d0) from [<c008d348>] (lock_acquire+0x4c/0x11c)
> [    0.241668] [<c008d2fc>] (lock_acquire+0x0/0x11c) from [<c0415aa4>] (_raw_spin_lock+0x3c/0x74)
> [    0.250610] [<c0415a68>] (_raw_spin_lock+0x0/0x74) from [<c010a844>] (set_task_comm+0x20/0xc0)
> [    0.259521]  r6:ee069588 r5:ee0691c0 r4:ee082000
> [    0.264404] [<c010a824>] (set_task_comm+0x0/0xc0) from [<c0060780>] (kthreadd+0x28/0x108)
> [    0.272857]  r8:00000000 r7:00000013 r6:c0044a08 r5:ee0691c0 r4:ee082000
> [    0.279571] r3:ee083fe0
> [    0.282470] [<c0060758>] (kthreadd+0x0/0x108) from [<c0044a08>] (do_exit+0x0/0x6dc)
> [    0.290405]  r5:c0060758 r4:00000000
> [    0.294189] ---[ end trace 1b75b31a2719ed1c ]---
> [    0.299041] possible reason: unannotated irqs-on.
> [    0.303955] irq event stamp: 5
> [    0.307159] hardirqs last  enabled at (4): [<c001331c>] no_work_pending+0x8/0x2c
> [    0.314880] hardirqs last disabled at (5): [<c0089b08>] trace_hardirqs_on_caller+0x60/0x26c
> [    0.323547] softirqs last  enabled at (0): [<c003f754>] copy_process+0x33c/0xef4
> [    0.331207] softirqs last disabled at (0): [<  (null)>]   (null)
> [    0.337585] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> 
> Acked-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

Applied to for-3.3-fixes.  Thank you.

-- 
tejun

  reply	other threads:[~2012-02-21 17:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-15  8:54 [PATCH v1] percpu: use raw_local_irq_* in _this_cpu op Ming Lei
2012-02-21 17:30 ` Tejun Heo [this message]
  -- strict thread matches above, loose matches on Subject: below --
2012-02-15  4:36 Ming Lei
2012-02-15  4:59 ` Eric Dumazet
2012-02-15  5:08   ` Ming Lei
2012-02-15  6:03     ` Eric Dumazet
2012-02-15  8:56       ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120221173042.GC12236@google.com \
    --to=tj@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tom.leiming@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.