From: Qian Cai <cai@lca.pw>
To: Peter Oberparleiter <oberpar@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Michal Hocko <mhocko@kernel.org>, Petr Mladek <pmladek@suse.com>
Cc: akpm@linux-foundation.org, sergey.senozhatsky.work@gmail.com,
rostedt@goodmis.org, peterz@infradead.org, linux-mm@kvack.org,
john.ogness@linutronix.de, david@redhat.com,
linux-kernel@vger.kernel.org,
Heiko Carstens <heiko.carstens@de.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [PATCH v2] mm/page_isolation: fix a deadlock with printk()
Date: Wed, 09 Oct 2019 11:25:38 -0400 [thread overview]
Message-ID: <1570634738.5937.12.camel@lca.pw> (raw)
In-Reply-To: <1157b3ae-006e-5b8e-71f0-883918992ecc@linux.ibm.com>
On Wed, 2019-10-09 at 15:56 +0200, Peter Oberparleiter wrote:
> On 08.10.2019 18:08, Qian Cai wrote:
> > On Tue, 2019-10-08 at 14:56 +0200, Christian Borntraeger wrote:
> > > Adding Peter Oberparleiter.
> > > Peter, can you have a look?
> > >
> > > On 08.10.19 10:27, Michal Hocko wrote:
> > > > On Tue 08-10-19 09:43:57, Petr Mladek wrote:
> > > > > On Mon 2019-10-07 16:49:37, Michal Hocko wrote:
> > > > > > [Cc s390 maintainers - the lockdep is http://lkml.kernel.org/r/1570228005-24979-1-git-send-email-cai@lca.pw
> > > > > > Petr has explained it is a false positive
> > > > > > http://lkml.kernel.org/r/20191007143002.l37bt2lzqtnqjqxu@pathway.suse.cz]
> > > > > > On Mon 07-10-19 16:30:02, Petr Mladek wrote:
> > > > > > [...]
> > > > > > > I believe that it cannot really happen because:
> > > > > > >
> > > > > > > static int __init
> > > > > > > sclp_console_init(void)
> > > > > > > {
> > > > > > > [...]
> > > > > > > rc = sclp_rw_init();
> > > > > > > [...]
> > > > > > > register_console(&sclp_console);
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > sclp_rw_init() is called before register_console(). And
> > > > > > > console_unlock() will never call sclp_console_write() before
> > > > > > > the console is registered.
> > > > > > >
> > > > > > > AFAIK, lockdep only compares existing chain of locks. It does
> > > > > > > not know about console registration that would make some
> > > > > > > code paths mutually exclusive.
> > > > > > >
> > > > > > > I believe that it is a false positive. I do not know how to
> > > > > > > avoid this lockdep report. I hope that it will disappear
> > > > > > > by deferring all printk() calls rather soon.
> > > > > >
> > > > > > Thanks a lot for looking into this Petr. I have also checked the code
> > > > > > and I really fail to see why the allocation has to be done under the
> > > > > > lock in the first place. sclp_read_sccb and sclp_init_sccb are global
> > > > > > variables but I strongly suspect that they need a synchronization during
> > > > > > early init, callbacks are registered only later IIUC:
> > > > >
> > > > > Good idea. It would work when the init function is called only once.
> > > > > But see below.
> > > > >
> > > > > > diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c
> > > > > > index d2ab3f07c008..4b1c033e3255 100644
> > > > > > --- a/drivers/s390/char/sclp.c
> > > > > > +++ b/drivers/s390/char/sclp.c
> > > > > > @@ -1169,13 +1169,13 @@ sclp_init(void)
> > > > > > unsigned long flags;
> > > > > > int rc = 0;
> > > > > >
> > > > > > + sclp_read_sccb = (void *) __get_free_page(GFP_ATOMIC | GFP_DMA);
> > > > > > + sclp_init_sccb = (void *) __get_free_page(GFP_ATOMIC | GFP_DMA);
> > > > > > spin_lock_irqsave(&sclp_lock, flags);
> > > > > > /* Check for previous or running initialization */
> > > > > > if (sclp_init_state != sclp_init_state_uninitialized)
> > > > > > goto fail_unlock;
> > > > >
> > > > > It seems that sclp_init() could be called several times in parallel.
> > > > > I see it called from sclp_register() and sclp_initcall().
> > > >
> > > > Interesting. Something for s390 people to answer I guess.
> > > > Anyway, this should be quite trivial to workaround by a cmpxch or alike.
> > > >
> >
> > The above fix is simply insufficient,
> >
> > 00: [ 3.654307] WARNING: possible circular locking dependency detected
> > 00: [ 3.654309] 5.4.0-rc1-next-20191004+ #4 Not tainted
> > 00: [ 3.654311] ------------------------------------------------------
> > 00: [ 3.654313] swapper/0/1 is trying to acquire lock:
> > 00: [ 3.654314] 00000000553f3fb8 (sclp_lock){-.-.}, at: sclp_add_request+0x34
> > 00: /0x308
> > 00: [ 3.654320]
> > 00: [ 3.654322] but task is already holding lock:
> > 00: [ 3.654323] 00000000550c9fc0 (console_owner){....}, at: console_unlock+0x
> > 00: 328/0xa30
> > 00: [ 3.654329]
> > 00: [ 3.654331] which lock already depends on the new lock.
>
> I can confirm that both this lockdep warning as well as the original one
> are both false positives: lockdep warns that code in sclp_init could
> trigger a deadlock via the chain
>
> sclp_lock --> &(&zone->lock)->rlock --> console_owner
>
> but
>
> a) before sclp_init successfully completes, register_console is not
> called, so there is no connection between console_owner -> sclp_lock
> b) after sclp_init completed, it won't be called again, so any
> dependency that requires a call-chain including sclp_init is
> impossible to achieve
>
> Apparently lockdep cannot determine that sclp_init won't be called again.
> I'm attaching a patch that moves sclp_init to __init so that lockdep has
> a chance of knowing that the function will be gone after init.
>
> This patch is intended for testing only though, to see if there are other
> paths to similar possible deadlocks. I still need to decide if its worth
> changing the code to remove false positives in lockdep.
>
> A generic solution would be preferable from my point of view though,
> because otherwise each console driver owner would need to ensure that any
> lock taken in their console.write implementation is never held while
> memory is allocated/released.
>
> diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c
> index d2ab3f07c008..13219e43d488 100644
> --- a/drivers/s390/char/sclp.c
> +++ b/drivers/s390/char/sclp.c
> @@ -140,7 +140,6 @@ static void sclp_request_timeout(bool force_restart);
> static void sclp_process_queue(void);
> static void __sclp_make_read_req(void);
> static int sclp_init_mask(int calculate);
> -static int sclp_init(void);
>
> static void
> __sclp_queue_read_req(void)
> @@ -670,7 +669,8 @@ __sclp_get_mask(sccb_mask_t *receive_mask, sccb_mask_t *send_mask)
> }
> }
>
> -/* Register event listener. Return 0 on success, non-zero otherwise. */
> +/* Register event listener. Return 0 on success, non-zero otherwise. Early
> + * callers (<= arch_initcall) must call sclp_init() first. */
> int
> sclp_register(struct sclp_register *reg)
> {
> @@ -679,9 +679,8 @@ sclp_register(struct sclp_register *reg)
> sccb_mask_t send_mask;
> int rc;
>
> - rc = sclp_init();
> - if (rc)
> - return rc;
> + if (sclp_init_state != sclp_init_state_initialized)
> + return -EINVAL;
> spin_lock_irqsave(&sclp_lock, flags);
> /* Check event mask for collisions */
> __sclp_get_mask(&receive_mask, &send_mask);
> @@ -1163,8 +1162,7 @@ static struct platform_device *sclp_pdev;
>
> /* Initialize SCLP driver. Return zero if driver is operational, non-zero
> * otherwise. */
> -static int
> -sclp_init(void)
> +int __init sclp_init(void)
> {
> unsigned long flags;
> int rc = 0;
> diff --git a/drivers/s390/char/sclp.h b/drivers/s390/char/sclp.h
> index 196333013e54..463660261379 100644
> --- a/drivers/s390/char/sclp.h
> +++ b/drivers/s390/char/sclp.h
> @@ -296,6 +296,7 @@ struct sclp_register {
> };
>
> /* externals from sclp.c */
> +int __init sclp_init(void);
> int sclp_add_request(struct sclp_req *req);
> void sclp_sync_wait(void);
> int sclp_register(struct sclp_register *reg);
> diff --git a/drivers/s390/char/sclp_con.c b/drivers/s390/char/sclp_con.c
> index 8966a1c1b548..a08ef2c8379e 100644
> --- a/drivers/s390/char/sclp_con.c
> +++ b/drivers/s390/char/sclp_con.c
> @@ -319,6 +319,9 @@ sclp_console_init(void)
> /* SCLP consoles are handled together */
> if (!(CONSOLE_IS_SCLP || CONSOLE_IS_VT220))
> return 0;
> + rc = sclp_init();
> + if (rc)
> + return rc;
> rc = sclp_rw_init();
> if (rc)
> return rc;
> diff --git a/drivers/s390/char/sclp_vt220.c b/drivers/s390/char/sclp_vt220.c
> index 3f9a6ef650fa..28b23e22248b 100644
> --- a/drivers/s390/char/sclp_vt220.c
> +++ b/drivers/s390/char/sclp_vt220.c
> @@ -694,6 +694,11 @@ static int __init __sclp_vt220_init(int num_pages)
> sclp_vt220_init_count++;
> if (sclp_vt220_init_count != 1)
> return 0;
> + rc = sclp_init();
> + if (rc) {
> + sclp_vt220_init_count--;
> + return rc;
> + }
> spin_lock_init(&sclp_vt220_lock);
> INIT_LIST_HEAD(&sclp_vt220_empty);
> INIT_LIST_HEAD(&sclp_vt220_outqueue);
Unfortunately, the patch does not help here. I guess the lockdep does not really
differential __init or not because those places can still have a chance to
deadlock in general after interrupt and preempt are enabled even during the boot
but it is just not the case here.
00: [ 2.812088] WARNING: possible circular locking dependency detected
00: [ 2.812090] 5.4.0-rc2-next-20191009+ #4 Not tainted
00: [ 2.812092] ------------------------------------------------------
00: [ 2.812094] swapper/0/1 is trying to acquire lock:
00: [ 2.812095] 0000000036a07ed8 (sclp_lock){-.-.}, at: sclp_add_request+0x34
00: /0x308
00: [ 2.812102]
00: [ 2.812103] but task is already holding lock:
00: [ 2.812105] 00000000366d9ec0 (console_owner){....}, at: console_unlock+0x
00: 328/0xa30
00: [ 2.812111]
00: [ 2.812113] which lock already depends on the new lock.
00: [ 2.812114]
00: [ 2.812115]
00: [ 2.812117] the existing dependency chain (in reverse order) is:
00: [ 2.812118]
00: [ 2.812120] -> #2 (console_owner){....}:
00: [ 2.812126] lock_acquire+0x21a/0x468
00: [ 2.812127] console_unlock+0x3a6/0xa30
00: [ 2.812129] vprintk_emit+0x184/0x3c8
00: [ 2.812131] vprintk_default+0x44/0x50
00: [ 2.812132] printk+0xa8/0xc0
00: [ 2.812134] get_random_u64+0x40/0x108
00: [ 2.812135] add_to_free_area_random+0x188/0x1c0
00: [ 2.812137] free_one_page+0x72/0x128
00: [ 2.812139] __free_pages_ok+0x51c/0xc90
00: [ 2.812141] memblock_free_all+0x30a/0x3b0
00: [ 2.812142] mem_init+0x84/0x200
00: [ 2.812144] start_kernel+0x384/0x6a0
00: [ 2.812145] startup_continue+0x70/0xd0
00: [ 2.812146]
00: [ 2.812147] -> #1 (&(&zone->lock)->rlock){....}:
00: [ 2.812154] lock_acquire+0x21a/0x468
00: [ 2.812155] _raw_spin_lock+0x54/0x68
00: [ 2.812157] get_page_from_freelist+0x8b6/0x2d28
00: [ 2.812159] __alloc_pages_nodemask+0x246/0x658
00: [ 2.812161] __get_free_pages+0x34/0x78
00: [ 2.812162] sclp_init+0xce/0x640
00: [ 2.812164] sclp_console_init+0x4e/0x1c0
00: [ 2.812166] console_init+0x2c8/0x410
00: [ 2.812168] start_kernel+0x530/0x6a0
00: [ 2.812169] startup_continue+0x70/0xd0
00: [ 2.812170]
00: [ 2.812171] -> #0 (sclp_lock){-.-.}:
00: [ 2.812177] check_noncircular+0x338/0x3e0
00: [ 2.812179] __lock_acquire+0x1e66/0x2d88
00: [ 2.812181] lock_acquire+0x21a/0x468
00: [ 2.812182] _raw_spin_lock_irqsave+0xcc/0xe8
00: [ 2.812184] sclp_add_request+0x34/0x308
00: [ 2.812186] sclp_conbuf_emit+0x100/0x138
00: [ 2.812187] sclp_console_write+0x96/0x3b8
00: [ 2.812189] console_unlock+0x6dc/0xa30
00: [ 2.812191] vprintk_emit+0x184/0x3c8
00: [ 2.812192] vprintk_default+0x44/0x50
00: [ 2.812194] printk+0xa8/0xc0
00: [ 2.812196] iommu_debugfs_setup+0xf2/0x108
00: [ 2.812198] iommu_init+0x6c/0x78
00: [ 2.812200] do_one_initcall+0x162/0x680
00: [ 2.812201] kernel_init_freeable+0x4e8/0x5a8
00: [ 2.812203] kernel_init+0x2a/0x188
00: [ 2.812205] ret_from_fork+0x30/0x34
00: [ 2.812206] kernel_thread_starter+0x0/0xc
00: [ 2.812207]
00: [ 2.812209] other info that might help us debug this:
00: [ 2.812210]
00: [ 2.812211] Chain exists of:
00: [ 2.812212] sclp_lock --> &(&zone->lock)->rlock --> console_owner
00: [ 2.812221]
00: [ 2.812222] Possible unsafe locking scenario:
00: [ 2.812223]
00: [ 2.812225] CPU0 CPU1
00: [ 2.812227] ---- ----
00: [ 2.812228] lock(console_owner);
00: [ 2.812232] lock(&(&zone->lock)->rlock);
00: [ 2.812236] lock(console_owner);
00: [ 2.812239] lock(sclp_lock);
00: [ 2.812243]
00: [ 2.812244] *** DEADLOCK ***
00: [ 2.812245]
00: [ 2.812247] 2 locks held by swapper/0/1:
00: [ 2.812248] #0: 00000000366da100 (console_lock){+.+.}, at: vprintk_emit+
00: 0x178/0x3c8
00: [ 2.812255] #1: 00000000366d9ec0 (console_owner){....}, at: console_unlo
00: ck+0x328/0xa30
00: [ 2.812262]
00: [ 2.812264] stack backtrace:
00: [ 2.812266] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc2-next-2019
00: 1009+ #4
00: [ 2.812268] Hardware name: IBM 2964 N96 400 (z/VM 6.4.0)
00: [ 2.812269] Call Trace:
00: [ 2.812271] ([<000000003591e230>] show_stack+0x110/0x1b0)
00: [ 2.812273] [<00000000361e025e>] dump_stack+0x126/0x178
00: [ 2.812276] [<0000000035a149b8>] check_noncircular+0x338/0x3e0
00: [ 2.812277] [<0000000035a1a9a6>] __lock_acquire+0x1e66/0x2d88
00: [ 2.812279] [<0000000035a17cc2>] lock_acquire+0x21a/0x468
00: [ 2.812281] [<000000003621bce4>] _raw_spin_lock_irqsave+0xcc/0xe8
00: [ 2.812283] [<0000000035ff534c>] sclp_add_request+0x34/0x308
00: [ 2.812285] [<0000000035ffc6b8>] sclp_conbuf_emit+0x100/0x138
00: [ 2.812287] [<0000000035ffc7d6>] sclp_console_write+0x96/0x3b8
00: [ 2.812289] [<0000000035a2b4fc>] console_unlock+0x6dc/0xa30
00: [ 2.812291] [<0000000035a2dd0c>] vprintk_emit+0x184/0x3c8
00: [ 2.812293] [<0000000035a2df94>] vprintk_default+0x44/0x50
00: [ 2.812295] [<0000000035a2ea40>] printk+0xa8/0xc0
00: [ 2.812297] [<0000000035f4e5ea>] iommu_debugfs_setup+0xf2/0x108
00: [ 2.812299] [<0000000036b90554>] iommu_init+0x6c/0x78
00: [ 2.812301] [<0000000035900fda>] do_one_initcall+0x162/0x680
00: [ 2.812303] [<0000000036b4f9f0>] kernel_init_freeable+0x4e8/0x5a8
00: [ 2.812305] [<0000000036206bda>] kernel_init+0x2a/0x188
00: [ 2.812306] [<000000003621cfdc>] ret_from_fork+0x30/0x34
00: [ 2.812308] [<000000003621cfe0>] kernel_thread_starter+0x0/0xc
00: [ 2.812310] INFO: lockdep is turned off.
WARNING: multiple messages have this Message-ID (diff)
From: "Qian Cai" <cai@lca.pw>
To: "Peter Oberparleiter" <oberpar@linux.ibm.com>,
"Christian Borntraeger" <borntraeger@de.ibm.com>,
"Michal Hocko" <mhocko@kernel.org>,
"Petr Mladek" <pmladek@suse.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"sergey.senozhatsky.work@gmail.com"
<sergey.senozhatsky.work@gmail.com>,
"rostedt@goodmis.org" <rostedt@goodmis.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"john.ogness@linutronix.de" <john.ogness@linutronix.de>,
"david@redhat.com" <david@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Heiko Carstens" <heiko.carstens@de.ibm.com>,
"Vasily Gorbik" <gor@linux.ibm.com>
Subject: Re: [PATCH v2] mm/page_isolation: fix a deadlock with printk()
Date: Wed, 09 Oct 2019 11:25:38 -0400 [thread overview]
Message-ID: <1570634738.5937.12.camel@lca.pw> (raw)
Message-ID: <20191009152538.E8wYrk9FTgzwctBk6ZL9bvPGA0Tml2QFYntGo2_wgOk@z> (raw)
In-Reply-To: <1157b3ae-006e-5b8e-71f0-883918992ecc@linux.ibm.com>
On Wed, 2019-10-09 at 15:56 +0200, Peter Oberparleiter wrote:
> On 08.10.2019 18:08, Qian Cai wrote:
> > On Tue, 2019-10-08 at 14:56 +0200, Christian Borntraeger wrote:
> > > Adding Peter Oberparleiter.
> > > Peter, can you have a look?
> > >
> > > On 08.10.19 10:27, Michal Hocko wrote:
> > > > On Tue 08-10-19 09:43:57, Petr Mladek wrote:
> > > > > On Mon 2019-10-07 16:49:37, Michal Hocko wrote:
> > > > > > [Cc s390 maintainers - the lockdep is http://lkml.kernel.org/r/1570228005-24979-1-git-send-email-cai@lca.pw
> > > > > > Petr has explained it is a false positive
> > > > > > http://lkml.kernel.org/r/20191007143002.l37bt2lzqtnqjqxu@pathway.suse.cz]
> > > > > > On Mon 07-10-19 16:30:02, Petr Mladek wrote:
> > > > > > [...]
> > > > > > > I believe that it cannot really happen because:
> > > > > > >
> > > > > > > static int __init
> > > > > > > sclp_console_init(void)
> > > > > > > {
> > > > > > > [...]
> > > > > > > rc = sclp_rw_init();
> > > > > > > [...]
> > > > > > > register_console(&sclp_console);
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > sclp_rw_init() is called before register_console(). And
> > > > > > > console_unlock() will never call sclp_console_write() before
> > > > > > > the console is registered.
> > > > > > >
> > > > > > > AFAIK, lockdep only compares existing chain of locks. It does
> > > > > > > not know about console registration that would make some
> > > > > > > code paths mutually exclusive.
> > > > > > >
> > > > > > > I believe that it is a false positive. I do not know how to
> > > > > > > avoid this lockdep report. I hope that it will disappear
> > > > > > > by deferring all printk() calls rather soon.
> > > > > >
> > > > > > Thanks a lot for looking into this Petr. I have also checked the code
> > > > > > and I really fail to see why the allocation has to be done under the
> > > > > > lock in the first place. sclp_read_sccb and sclp_init_sccb are global
> > > > > > variables but I strongly suspect that they need a synchronization during
> > > > > > early init, callbacks are registered only later IIUC:
> > > > >
> > > > > Good idea. It would work when the init function is called only once.
> > > > > But see below.
> > > > >
> > > > > > diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c
> > > > > > index d2ab3f07c008..4b1c033e3255 100644
> > > > > > --- a/drivers/s390/char/sclp.c
> > > > > > +++ b/drivers/s390/char/sclp.c
> > > > > > @@ -1169,13 +1169,13 @@ sclp_init(void)
> > > > > > unsigned long flags;
> > > > > > int rc = 0;
> > > > > >
> > > > > > + sclp_read_sccb = (void *) __get_free_page(GFP_ATOMIC | GFP_DMA);
> > > > > > + sclp_init_sccb = (void *) __get_free_page(GFP_ATOMIC | GFP_DMA);
> > > > > > spin_lock_irqsave(&sclp_lock, flags);
> > > > > > /* Check for previous or running initialization */
> > > > > > if (sclp_init_state != sclp_init_state_uninitialized)
> > > > > > goto fail_unlock;
> > > > >
> > > > > It seems that sclp_init() could be called several times in parallel.
> > > > > I see it called from sclp_register() and sclp_initcall().
> > > >
> > > > Interesting. Something for s390 people to answer I guess.
> > > > Anyway, this should be quite trivial to workaround by a cmpxch or alike.
> > > >
> >
> > The above fix is simply insufficient,
> >
> > 00: [ 3.654307] WARNING: possible circular locking dependency detected
> > 00: [ 3.654309] 5.4.0-rc1-next-20191004+ #4 Not tainted
> > 00: [ 3.654311] ------------------------------------------------------
> > 00: [ 3.654313] swapper/0/1 is trying to acquire lock:
> > 00: [ 3.654314] 00000000553f3fb8 (sclp_lock){-.-.}, at: sclp_add_request+0x34
> > 00: /0x308
> > 00: [ 3.654320]
> > 00: [ 3.654322] but task is already holding lock:
> > 00: [ 3.654323] 00000000550c9fc0 (console_owner){....}, at: console_unlock+0x
> > 00: 328/0xa30
> > 00: [ 3.654329]
> > 00: [ 3.654331] which lock already depends on the new lock.
>
> I can confirm that both this lockdep warning as well as the original one
> are both false positives: lockdep warns that code in sclp_init could
> trigger a deadlock via the chain
>
> sclp_lock --> &(&zone->lock)->rlock --> console_owner
>
> but
>
> a) before sclp_init successfully completes, register_console is not
> called, so there is no connection between console_owner -> sclp_lock
> b) after sclp_init completed, it won't be called again, so any
> dependency that requires a call-chain including sclp_init is
> impossible to achieve
>
> Apparently lockdep cannot determine that sclp_init won't be called again.
> I'm attaching a patch that moves sclp_init to __init so that lockdep has
> a chance of knowing that the function will be gone after init.
>
> This patch is intended for testing only though, to see if there are other
> paths to similar possible deadlocks. I still need to decide if its worth
> changing the code to remove false positives in lockdep.
>
> A generic solution would be preferable from my point of view though,
> because otherwise each console driver owner would need to ensure that any
> lock taken in their console.write implementation is never held while
> memory is allocated/released.
>
> diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c
> index d2ab3f07c008..13219e43d488 100644
> --- a/drivers/s390/char/sclp.c
> +++ b/drivers/s390/char/sclp.c
> @@ -140,7 +140,6 @@ static void sclp_request_timeout(bool force_restart);
> static void sclp_process_queue(void);
> static void __sclp_make_read_req(void);
> static int sclp_init_mask(int calculate);
> -static int sclp_init(void);
>
> static void
> __sclp_queue_read_req(void)
> @@ -670,7 +669,8 @@ __sclp_get_mask(sccb_mask_t *receive_mask, sccb_mask_t *send_mask)
> }
> }
>
> -/* Register event listener. Return 0 on success, non-zero otherwise. */
> +/* Register event listener. Return 0 on success, non-zero otherwise. Early
> + * callers (<= arch_initcall) must call sclp_init() first. */
> int
> sclp_register(struct sclp_register *reg)
> {
> @@ -679,9 +679,8 @@ sclp_register(struct sclp_register *reg)
> sccb_mask_t send_mask;
> int rc;
>
> - rc = sclp_init();
> - if (rc)
> - return rc;
> + if (sclp_init_state != sclp_init_state_initialized)
> + return -EINVAL;
> spin_lock_irqsave(&sclp_lock, flags);
> /* Check event mask for collisions */
> __sclp_get_mask(&receive_mask, &send_mask);
> @@ -1163,8 +1162,7 @@ static struct platform_device *sclp_pdev;
>
> /* Initialize SCLP driver. Return zero if driver is operational, non-zero
> * otherwise. */
> -static int
> -sclp_init(void)
> +int __init sclp_init(void)
> {
> unsigned long flags;
> int rc = 0;
> diff --git a/drivers/s390/char/sclp.h b/drivers/s390/char/sclp.h
> index 196333013e54..463660261379 100644
> --- a/drivers/s390/char/sclp.h
> +++ b/drivers/s390/char/sclp.h
> @@ -296,6 +296,7 @@ struct sclp_register {
> };
>
> /* externals from sclp.c */
> +int __init sclp_init(void);
> int sclp_add_request(struct sclp_req *req);
> void sclp_sync_wait(void);
> int sclp_register(struct sclp_register *reg);
> diff --git a/drivers/s390/char/sclp_con.c b/drivers/s390/char/sclp_con.c
> index 8966a1c1b548..a08ef2c8379e 100644
> --- a/drivers/s390/char/sclp_con.c
> +++ b/drivers/s390/char/sclp_con.c
> @@ -319,6 +319,9 @@ sclp_console_init(void)
> /* SCLP consoles are handled together */
> if (!(CONSOLE_IS_SCLP || CONSOLE_IS_VT220))
> return 0;
> + rc = sclp_init();
> + if (rc)
> + return rc;
> rc = sclp_rw_init();
> if (rc)
> return rc;
> diff --git a/drivers/s390/char/sclp_vt220.c b/drivers/s390/char/sclp_vt220.c
> index 3f9a6ef650fa..28b23e22248b 100644
> --- a/drivers/s390/char/sclp_vt220.c
> +++ b/drivers/s390/char/sclp_vt220.c
> @@ -694,6 +694,11 @@ static int __init __sclp_vt220_init(int num_pages)
> sclp_vt220_init_count++;
> if (sclp_vt220_init_count != 1)
> return 0;
> + rc = sclp_init();
> + if (rc) {
> + sclp_vt220_init_count--;
> + return rc;
> + }
> spin_lock_init(&sclp_vt220_lock);
> INIT_LIST_HEAD(&sclp_vt220_empty);
> INIT_LIST_HEAD(&sclp_vt220_outqueue);
Unfortunately, the patch does not help here. I guess the lockdep does not really
differential __init or not because those places can still have a chance to
deadlock in general after interrupt and preempt are enabled even during the boot
but it is just not the case here.
00: [ 2.812088] WARNING: possible circular locking dependency detected
00: [ 2.812090] 5.4.0-rc2-next-20191009+ #4 Not tainted
00: [ 2.812092] ------------------------------------------------------
00: [ 2.812094] swapper/0/1 is trying to acquire lock:
00: [ 2.812095] 0000000036a07ed8 (sclp_lock){-.-.}, at: sclp_add_request+0x34
00: /0x308
00: [ 2.812102]
00: [ 2.812103] but task is already holding lock:
00: [ 2.812105] 00000000366d9ec0 (console_owner){....}, at: console_unlock+0x
00: 328/0xa30
00: [ 2.812111]
00: [ 2.812113] which lock already depends on the new lock.
00: [ 2.812114]
00: [ 2.812115]
00: [ 2.812117] the existing dependency chain (in reverse order) is:
00: [ 2.812118]
00: [ 2.812120] -> #2 (console_owner){....}:
00: [ 2.812126] lock_acquire+0x21a/0x468
00: [ 2.812127] console_unlock+0x3a6/0xa30
00: [ 2.812129] vprintk_emit+0x184/0x3c8
00: [ 2.812131] vprintk_default+0x44/0x50
00: [ 2.812132] printk+0xa8/0xc0
00: [ 2.812134] get_random_u64+0x40/0x108
00: [ 2.812135] add_to_free_area_random+0x188/0x1c0
00: [ 2.812137] free_one_page+0x72/0x128
00: [ 2.812139] __free_pages_ok+0x51c/0xc90
00: [ 2.812141] memblock_free_all+0x30a/0x3b0
00: [ 2.812142] mem_init+0x84/0x200
00: [ 2.812144] start_kernel+0x384/0x6a0
00: [ 2.812145] startup_continue+0x70/0xd0
00: [ 2.812146]
00: [ 2.812147] -> #1 (&(&zone->lock)->rlock){....}:
00: [ 2.812154] lock_acquire+0x21a/0x468
00: [ 2.812155] _raw_spin_lock+0x54/0x68
00: [ 2.812157] get_page_from_freelist+0x8b6/0x2d28
00: [ 2.812159] __alloc_pages_nodemask+0x246/0x658
00: [ 2.812161] __get_free_pages+0x34/0x78
00: [ 2.812162] sclp_init+0xce/0x640
00: [ 2.812164] sclp_console_init+0x4e/0x1c0
00: [ 2.812166] console_init+0x2c8/0x410
00: [ 2.812168] start_kernel+0x530/0x6a0
00: [ 2.812169] startup_continue+0x70/0xd0
00: [ 2.812170]
00: [ 2.812171] -> #0 (sclp_lock){-.-.}:
00: [ 2.812177] check_noncircular+0x338/0x3e0
00: [ 2.812179] __lock_acquire+0x1e66/0x2d88
00: [ 2.812181] lock_acquire+0x21a/0x468
00: [ 2.812182] _raw_spin_lock_irqsave+0xcc/0xe8
00: [ 2.812184] sclp_add_request+0x34/0x308
00: [ 2.812186] sclp_conbuf_emit+0x100/0x138
00: [ 2.812187] sclp_console_write+0x96/0x3b8
00: [ 2.812189] console_unlock+0x6dc/0xa30
00: [ 2.812191] vprintk_emit+0x184/0x3c8
00: [ 2.812192] vprintk_default+0x44/0x50
00: [ 2.812194] printk+0xa8/0xc0
00: [ 2.812196] iommu_debugfs_setup+0xf2/0x108
00: [ 2.812198] iommu_init+0x6c/0x78
00: [ 2.812200] do_one_initcall+0x162/0x680
00: [ 2.812201] kernel_init_freeable+0x4e8/0x5a8
00: [ 2.812203] kernel_init+0x2a/0x188
00: [ 2.812205] ret_from_fork+0x30/0x34
00: [ 2.812206] kernel_thread_starter+0x0/0xc
00: [ 2.812207]
00: [ 2.812209] other info that might help us debug this:
00: [ 2.812210]
00: [ 2.812211] Chain exists of:
00: [ 2.812212] sclp_lock --> &(&zone->lock)->rlock --> console_owner
00: [ 2.812221]
00: [ 2.812222] Possible unsafe locking scenario:
00: [ 2.812223]
00: [ 2.812225] CPU0 CPU1
00: [ 2.812227] ---- ----
00: [ 2.812228] lock(console_owner);
00: [ 2.812232] lock(&(&zone->lock)->rlock);
00: [ 2.812236] lock(console_owner);
00: [ 2.812239] lock(sclp_lock);
00: [ 2.812243]
00: [ 2.812244] *** DEADLOCK ***
00: [ 2.812245]
00: [ 2.812247] 2 locks held by swapper/0/1:
00: [ 2.812248] #0: 00000000366da100 (console_lock){+.+.}, at: vprintk_emit+
00: 0x178/0x3c8
00: [ 2.812255] #1: 00000000366d9ec0 (console_owner){....}, at: console_unlo
00: ck+0x328/0xa30
00: [ 2.812262]
00: [ 2.812264] stack backtrace:
00: [ 2.812266] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc2-next-2019
00: 1009+ #4
00: [ 2.812268] Hardware name: IBM 2964 N96 400 (z/VM 6.4.0)
00: [ 2.812269] Call Trace:
00: [ 2.812271] ([<000000003591e230>] show_stack+0x110/0x1b0)
00: [ 2.812273] [<00000000361e025e>] dump_stack+0x126/0x178
00: [ 2.812276] [<0000000035a149b8>] check_noncircular+0x338/0x3e0
00: [ 2.812277] [<0000000035a1a9a6>] __lock_acquire+0x1e66/0x2d88
00: [ 2.812279] [<0000000035a17cc2>] lock_acquire+0x21a/0x468
00: [ 2.812281] [<000000003621bce4>] _raw_spin_lock_irqsave+0xcc/0xe8
00: [ 2.812283] [<0000000035ff534c>] sclp_add_request+0x34/0x308
00: [ 2.812285] [<0000000035ffc6b8>] sclp_conbuf_emit+0x100/0x138
00: [ 2.812287] [<0000000035ffc7d6>] sclp_console_write+0x96/0x3b8
00: [ 2.812289] [<0000000035a2b4fc>] console_unlock+0x6dc/0xa30
00: [ 2.812291] [<0000000035a2dd0c>] vprintk_emit+0x184/0x3c8
00: [ 2.812293] [<0000000035a2df94>] vprintk_default+0x44/0x50
00: [ 2.812295] [<0000000035a2ea40>] printk+0xa8/0xc0
00: [ 2.812297] [<0000000035f4e5ea>] iommu_debugfs_setup+0xf2/0x108
00: [ 2.812299] [<0000000036b90554>] iommu_init+0x6c/0x78
00: [ 2.812301] [<0000000035900fda>] do_one_initcall+0x162/0x680
00: [ 2.812303] [<0000000036b4f9f0>] kernel_init_freeable+0x4e8/0x5a8
00: [ 2.812305] [<0000000036206bda>] kernel_init+0x2a/0x188
00: [ 2.812306] [<000000003621cfdc>] ret_from_fork+0x30/0x34
00: [ 2.812308] [<000000003621cfe0>] kernel_thread_starter+0x0/0xc
00: [ 2.812310] INFO: lockdep is turned off.
next prev parent reply other threads:[~2019-10-09 15:25 UTC|newest]
Thread overview: 97+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 22:26 [PATCH v2] mm/page_isolation: fix a deadlock with printk() Qian Cai
2019-10-07 8:07 ` Michal Hocko
2019-10-07 9:05 ` Petr Mladek
2019-10-07 11:33 ` Michal Hocko
2019-10-07 12:34 ` Qian Cai
2019-10-07 12:34 ` Qian Cai
2019-10-07 11:04 ` Qian Cai
2019-10-07 11:37 ` Michal Hocko
2019-10-07 12:11 ` Qian Cai
2019-10-07 12:11 ` Qian Cai
2019-10-07 12:43 ` Michal Hocko
2019-10-07 13:07 ` Qian Cai
2019-10-07 13:07 ` Qian Cai
2019-10-07 14:10 ` Petr Mladek
2019-10-07 14:30 ` Petr Mladek
2019-10-07 14:49 ` Michal Hocko
2019-10-08 7:43 ` Petr Mladek
2019-10-08 8:27 ` Michal Hocko
2019-10-08 12:56 ` Christian Borntraeger
2019-10-08 16:08 ` Qian Cai
2019-10-08 16:08 ` Qian Cai
2019-10-08 18:35 ` Michal Hocko
2019-10-08 19:06 ` Qian Cai
2019-10-08 19:06 ` Qian Cai
2019-10-08 19:17 ` Michal Hocko
2019-10-08 19:35 ` Qian Cai
2019-10-08 19:35 ` Qian Cai
2019-10-09 11:49 ` Petr Mladek
2019-10-09 13:06 ` Qian Cai
2019-10-09 13:06 ` Qian Cai
2019-10-09 13:27 ` Michal Hocko
2019-10-09 13:43 ` Qian Cai
2019-10-09 13:43 ` Qian Cai
2019-10-09 13:51 ` Michal Hocko
2019-10-09 14:19 ` Qian Cai
2019-10-09 14:19 ` Qian Cai
2019-10-09 14:34 ` Michal Hocko
2019-10-09 15:08 ` Qian Cai
2019-10-09 15:08 ` Qian Cai
2019-10-09 16:23 ` Michal Hocko
2019-10-09 16:23 ` Michal Hocko
2019-10-10 9:01 ` Qian Cai
2019-10-10 10:59 ` Michal Hocko
2019-10-10 13:11 ` Qian Cai
2019-10-10 13:11 ` Qian Cai
2019-10-10 14:18 ` Michal Hocko
2019-10-10 14:47 ` Qian Cai
2019-10-10 14:47 ` Qian Cai
2019-10-10 17:30 ` Michal Hocko
2019-10-10 17:48 ` Qian Cai
2019-10-10 17:48 ` Qian Cai
2019-10-10 18:06 ` Michal Hocko
2019-10-10 18:59 ` David Hildenbrand
2019-10-09 14:24 ` Petr Mladek
2019-10-09 14:46 ` Qian Cai
2019-10-09 14:46 ` Qian Cai
2019-10-10 7:57 ` Petr Mladek
2019-10-09 11:39 ` Petr Mladek
2019-10-09 13:56 ` Peter Oberparleiter
2019-10-09 14:26 ` Michal Hocko
2019-10-10 5:12 ` Sergey Senozhatsky
2019-10-10 7:40 ` Michal Hocko
2019-10-10 8:16 ` Sergey Senozhatsky
2019-10-10 8:37 ` Michal Hocko
2019-10-10 8:21 ` Petr Mladek
2019-10-10 8:39 ` Sergey Senozhatsky
2019-10-10 11:11 ` Petr Mladek
2019-10-09 15:25 ` Qian Cai [this message]
2019-10-09 15:25 ` Qian Cai
2019-10-09 15:25 ` Qian Cai
2019-10-07 14:59 ` Qian Cai
2019-10-07 14:59 ` Qian Cai
2019-10-07 15:12 ` Michal Hocko
2019-10-07 15:33 ` Qian Cai
2019-10-07 15:33 ` Qian Cai
2019-10-08 8:15 ` Petr Mladek
2019-10-08 9:32 ` Qian Cai
2019-10-08 13:13 ` Steven Rostedt
2019-10-08 13:23 ` Qian Cai
2019-10-08 13:23 ` Qian Cai
2019-10-08 13:33 ` Steven Rostedt
2019-10-08 13:42 ` Petr Mladek
2019-10-08 13:48 ` Michal Hocko
2019-10-08 14:03 ` Qian Cai
2019-10-08 14:03 ` Qian Cai
2019-10-08 14:08 ` Michal Hocko
2019-10-08 8:40 ` Michal Hocko
2019-10-08 10:04 ` Qian Cai
2019-10-08 10:39 ` Michal Hocko
2019-10-08 12:00 ` Qian Cai
2019-10-08 12:39 ` Michal Hocko
2019-10-08 13:06 ` Qian Cai
2019-10-08 13:06 ` Qian Cai
2019-10-08 13:37 ` Michal Hocko
2019-10-08 13:08 ` Petr Mladek
2019-10-08 13:33 ` Qian Cai
2019-10-08 13:33 ` Qian Cai
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=1570634738.5937.12.camel@lca.pw \
--to=cai@lca.pw \
--cc=akpm@linux-foundation.org \
--cc=borntraeger@de.ibm.com \
--cc=david@redhat.com \
--cc=gor@linux.ibm.com \
--cc=heiko.carstens@de.ibm.com \
--cc=john.ogness@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=oberpar@linux.ibm.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@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.