linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@kernel.org, tglx@linutronix.de, walken@google.com,
	boqun.feng@gmail.com, kirill@shutemov.name,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, willy@infradead.org,
	npiggin@gmail.com, kernel-team@lge.com
Subject: Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature
Date: Thu, 10 Aug 2017 12:47:18 +0900	[thread overview]
Message-ID: <20170810034718.GZ20323@X58A-UD3R> (raw)
In-Reply-To: <20170810005556.GU20323@X58A-UD3R>

On Thu, Aug 10, 2017 at 09:55:56AM +0900, Byungchul Park wrote:
> On Wed, Aug 09, 2017 at 05:50:59PM +0200, Peter Zijlstra wrote:
> > 
> > 
> > Heh, look what it does...
> 
> It does not happen in my machine..
> 
> I tihink it happens because of "Simplify xhlock ring buffer invalidation"
> patch of you.
> 
> First of all, could you reverse yours and check if it happens, too?
> If not, we have to think the simplification more.
> 
> BTW, does your patch consider the possibility that a worker and irqs can
> be nested? Is it no problem even in the case?

In addition, now that each syscall context is isolated by your suggestion
with crossrelease_hist_end() and crossrelease_hist_start(), contexts can
be nested easily. I want to keep my patches unchanged at first and change
code carefully.

> 
> > 
> > 
> > 4======================================================
> > 4WARNING: possible circular locking dependency detected
> > 4.13.0-rc2-00317-gadc6764a3adf-dirty #797 Tainted: G        W      
> > 4------------------------------------------------------
> > 4startpar/582 is trying to acquire lock:
> > c (c(complete)&barr->donec){+.+.}c, at: [<ffffffff8110de4d>] flush_work+0x1fd/0x2c0
> > 4
> > but task is already holding lock:
> > c (clockc#3c){+.+.}c, at: [<ffffffff8122e866>] lru_add_drain_all_cpuslocked+0x46/0x1a0
> > 4
> > which lock already depends on the new lock.
> > 
> > 4
> > the existing dependency chain (in reverse order) is:
> > 
> > -> #4c (clockc#3c){+.+.}c:
> >        __lock_acquire+0x10a5/0x1100
> >        lock_acquire+0xea/0x1f0
> >        __mutex_lock+0x6c/0x960
> >        mutex_lock_nested+0x1b/0x20
> >        lru_add_drain_all_cpuslocked+0x46/0x1a0
> >        lru_add_drain_all+0x13/0x20
> >        SyS_mlockall+0xb8/0x1c0
> >        entry_SYSCALL_64_fastpath+0x23/0xc2
> > 
> > -> #3c (ccpu_hotplug_lock.rw_semc){++++}c:
> >        __lock_acquire+0x10a5/0x1100
> >        lock_acquire+0xea/0x1f0
> >        cpus_read_lock+0x2a/0x90
> >        kmem_cache_create+0x2a/0x1d0
> >        scsi_init_sense_cache+0xa0/0xc0
> >        scsi_add_host_with_dma+0x67/0x360
> >        isci_pci_probe+0x873/0xc90
> >        local_pci_probe+0x42/0xa0
> >        work_for_cpu_fn+0x14/0x20
> >        process_one_work+0x273/0x6b0
> >        worker_thread+0x21b/0x3f0
> >        kthread+0x147/0x180
> >        ret_from_fork+0x2a/0x40
> > 
> > -> #2c (cscsi_sense_cache_mutexc){+.+.}c:
> >        __lock_acquire+0x10a5/0x1100
> >        lock_acquire+0xea/0x1f0
> >        __mutex_lock+0x6c/0x960
> >        mutex_lock_nested+0x1b/0x20
> >        scsi_init_sense_cache+0x3d/0xc0
> >        scsi_add_host_with_dma+0x67/0x360
> >        isci_pci_probe+0x873/0xc90
> >        local_pci_probe+0x42/0xa0
> >        work_for_cpu_fn+0x14/0x20
> >        process_one_work+0x273/0x6b0
> >        worker_thread+0x21b/0x3f0
> >        kthread+0x147/0x180
> >        ret_from_fork+0x2a/0x40
> > 
> > -> #1c (c(&wfc.work)c){+.+.}c:
> >        process_one_work+0x244/0x6b0
> >        worker_thread+0x21b/0x3f0
> >        kthread+0x147/0x180
> >        ret_from_fork+0x2a/0x40
> >        0xffffffffffffffff
> > 
> > -> #0c (c(complete)&barr->donec){+.+.}c:
> >        check_prev_add+0x3be/0x700
> >        __lock_acquire+0x10a5/0x1100
> >        lock_acquire+0xea/0x1f0
> >        wait_for_completion+0x3b/0x130
> >        flush_work+0x1fd/0x2c0
> >        lru_add_drain_all_cpuslocked+0x158/0x1a0
> >        lru_add_drain_all+0x13/0x20
> >        SyS_mlockall+0xb8/0x1c0
> >        entry_SYSCALL_64_fastpath+0x23/0xc2
> > 
> > other info that might help us debug this:
> > 
> > Chain exists of:
> >   c(complete)&barr->donec --> ccpu_hotplug_lock.rw_semc --> clockc#3c
> > 
> >  Possible unsafe locking scenario:
> > 
> >        CPU0                    CPU1
> >        ----                    ----
> >   lock(clockc#3c);
> >                                lock(ccpu_hotplug_lock.rw_semc);
> >                                lock(clockc#3c);
> >   lock(c(complete)&barr->donec);
> > 
> >  *** DEADLOCK ***
> > 
> > 2 locks held by startpar/582:
> >  #0: c (ccpu_hotplug_lock.rw_semc){++++}c, at: [<ffffffff8122e9ce>] lru_add_drain_all+0xe/0x20
> >  #1: c (clockc#3c){+.+.}c, at: [<ffffffff8122e866>] lru_add_drain_all_cpuslocked+0x46/0x1a0
> > 
> > stack backtrace:
> > dCPU: 23 PID: 582 Comm: startpar Tainted: G        W       4.13.0-rc2-00317-gadc6764a3adf-dirty #797
> > dHardware name: Intel Corporation S2600GZ/S2600GZ, BIOS SE5C600.86B.02.02.0002.122320131210 12/23/2013
> > dCall Trace:
> > d dump_stack+0x86/0xcf
> > d print_circular_bug+0x203/0x2f0
> > d check_prev_add+0x3be/0x700
> > d ? add_lock_to_list.isra.30+0xc0/0xc0
> > d ? is_bpf_text_address+0x82/0xe0
> > d ? unwind_get_return_address+0x1f/0x30
> > d __lock_acquire+0x10a5/0x1100
> > d ? __lock_acquire+0x10a5/0x1100
> > d ? add_lock_to_list.isra.30+0xc0/0xc0
> > d lock_acquire+0xea/0x1f0
> > d ? flush_work+0x1fd/0x2c0
> > d wait_for_completion+0x3b/0x130
> > d ? flush_work+0x1fd/0x2c0
> > d flush_work+0x1fd/0x2c0
> > d ? flush_workqueue_prep_pwqs+0x1c0/0x1c0
> > d ? trace_hardirqs_on+0xd/0x10
> > d lru_add_drain_all_cpuslocked+0x158/0x1a0
> > d lru_add_drain_all+0x13/0x20
> > d SyS_mlockall+0xb8/0x1c0
> > d entry_SYSCALL_64_fastpath+0x23/0xc2
> > dRIP: 0033:0x7f818d2e54c7
> > dRSP: 002b:00007fffcce83798 EFLAGS: 00000246c ORIG_RAX: 0000000000000097
> > dRAX: ffffffffffffffda RBX: 0000000000000046 RCX: 00007f818d2e54c7
> > dRDX: 0000000000000000 RSI: 00007fffcce83650 RDI: 0000000000000003
> > dRBP: 000000000002c010 R08: 0000000000000000 R09: 0000000000000000
> > dR10: 0000000000000008 R11: 0000000000000246 R12: 000000000002d000
> > dR13: 000000000002c010 R14: 0000000000001000 R15: 00007f818d599b00

  reply	other threads:[~2017-08-10  3:48 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-07  7:12 [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park
2017-08-07  7:12 ` [PATCH v8 01/14] lockdep: Refactor lookup_chain_cache() Byungchul Park
2017-08-10 12:18   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 02/14] lockdep: Add a function building a chain between two classes Byungchul Park
2017-08-10 12:18   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 03/14] lockdep: Change the meaning of check_prev_add()'s return value Byungchul Park
2017-08-10 12:19   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 04/14] lockdep: Make check_prev_add() able to handle external stack_trace Byungchul Park
2017-08-10 12:19   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 05/14] lockdep: Implement crossrelease feature Byungchul Park
2017-08-09 14:05   ` Peter Zijlstra
2017-08-10  1:30     ` Byungchul Park
2017-08-10  9:21       ` Peter Zijlstra
2017-08-10 12:19   ` [tip:locking/core] locking/lockdep: Implement the 'crossrelease' feature tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 06/14] lockdep: Detect and handle hist_lock ring buffer overwrite Byungchul Park
2017-08-09 14:16   ` Peter Zijlstra
2017-08-10  1:32     ` Byungchul Park
2017-08-10  9:22       ` Peter Zijlstra
2017-08-10 10:32     ` Byungchul Park
2017-08-10 11:59   ` Boqun Feng
2017-08-10 12:11     ` Byungchul Park
2017-08-10 12:51       ` Boqun Feng
2017-08-10 13:17         ` Boqun Feng
2017-08-11  0:44           ` Byungchul Park
2017-08-11  3:43           ` Byungchul Park
2017-08-11  8:03             ` Boqun Feng
2017-08-11  8:52               ` Byungchul Park
2017-08-11  9:44                 ` Byungchul Park
2017-08-11 13:06                   ` Byungchul Park
2017-08-14  7:05                     ` Boqun Feng
2017-08-14  7:22                       ` Byungchul Park
2017-08-14  7:29                       ` Byungchul Park
2017-08-11  0:40         ` Byungchul Park
2017-08-11  1:03           ` Boqun Feng
2017-08-10 12:20   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 07/14] lockdep: Handle non(or multi)-acquisition of a crosslock Byungchul Park
2017-08-10 12:20   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 08/14] lockdep: Make print_circular_bug() aware of crossrelease Byungchul Park
2017-08-10 12:21   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-07  7:12 ` [PATCH v8 09/14] lockdep: Apply crossrelease to completions Byungchul Park
2017-08-07 10:20   ` kbuild test robot
2017-08-07 11:45   ` kbuild test robot
2017-08-09  9:51   ` Peter Zijlstra
2017-08-09 10:24     ` Peter Zijlstra
2017-08-10  1:24       ` Byungchul Park
2017-08-10 12:21   ` [tip:locking/core] locking/lockdep: " tip-bot for Byungchul Park
2017-08-14  8:50   ` [PATCH v8 09/14] lockdep: " Arnd Bergmann
2017-08-18 23:43     ` Boqun Feng
2017-08-19 12:51       ` Arnd Bergmann
2017-08-19 13:34         ` Arnd Bergmann
2017-08-23 14:43           ` Boqun Feng
2017-08-20  3:18         ` Boqun Feng
2017-08-07  7:12 ` [PATCH v8 10/14] pagemap.h: Remove trailing white space Byungchul Park
2017-08-07  7:12 ` [PATCH v8 11/14] lockdep: Apply crossrelease to PG_locked locks Byungchul Park
2017-08-07 10:36   ` kbuild test robot
2017-08-10  1:35   ` Byungchul Park
2017-08-10  9:25     ` Peter Zijlstra
2017-09-05  1:03   ` Byungchul Park
2017-08-07  7:12 ` [PATCH v8 12/14] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked Byungchul Park
2017-08-07  7:13 ` [PATCH v8 13/14] lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext Byungchul Park
2017-08-07 10:43   ` kbuild test robot
2017-08-07  7:13 ` [PATCH v8 14/14] lockdep: Crossrelease feature documentation Byungchul Park
2017-08-07 15:58   ` kbuild test robot
2017-08-10 12:22   ` [tip:locking/core] locking/lockdep: Add 'crossrelease' " tip-bot for Byungchul Park
2017-08-09 15:50 ` [PATCH v8 00/14] lockdep: Implement crossrelease feature Peter Zijlstra
2017-08-10  0:55   ` Byungchul Park
2017-08-10  3:47     ` Byungchul Park [this message]
2017-08-10 10:52     ` Byungchul Park
2017-08-10  9:37   ` Byungchul Park
2017-08-10 10:52     ` Peter Zijlstra
2017-08-10 11:10 ` Ingo Molnar
2017-08-10 11:45   ` Byungchul Park
2017-08-14 10:57     ` Ingo Molnar
2017-08-14 11:10       ` Byungchul Park
2017-08-15  8:20 ` Ingo Molnar
2017-08-16  0:16   ` Byungchul Park
2017-08-16  4:05     ` Boqun Feng
2017-08-16  4:37       ` Byungchul Park
2017-08-16  5:40         ` Boqun Feng
2017-08-16  6:37           ` Byungchul Park
2017-08-16  5:05       ` Byungchul Park
2017-08-16  5:58         ` Boqun Feng
2017-08-16  7:14           ` Byungchul Park
2017-08-16  8:06             ` Byungchul Park
2017-08-16  9:38               ` Byungchul Park
2017-08-17  7:48       ` Ingo Molnar
2017-08-17  8:04         ` Boqun Feng
2017-08-17  8:12           ` Ingo Molnar
2017-08-17  8:33             ` Boqun Feng

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=20170810034718.GZ20323@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=kernel-team@lge.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=walken@google.com \
    --cc=willy@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).