Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 1DsSKYx08gBQZ38Mru32PSrKDgGW69i1oMBK+IoSEkFYWLBxzDWJthZz113bqAeB+iBUb+Ywg7
 zGSaCxqHA9LshOxk2hTfrMEoX5wMO/D3Te+EqBf8+FEDYwxpwpXSSW7IPzjIbqVMok7ef5btwb
 qrafxIrkDxWGnuK/eGMdI2LKkZogUGI70zUyJ13de7I+G3686L4lZSsjvmJROW2gjrWlm8llMI
 55IhV7bW+PDClj3ppUu0gYzkD7b09hmzaCvGa79RRSRasnQriXd6rsqadBbgdC/qLJMHvEbkpc
 2Hg=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Logan Gunthorpe <logang@deltatee.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Keith Busch <kbusch@kernel.org>
CC:     Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
        Keith Busch <Keith.Busch@wdc.com>, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH v2] nvme: Use spin_lock_irq() when taking the ctrl->lock
Thread-Topic: [PATCH v2] nvme: Use spin_lock_irq() when taking the ctrl->lock
Thread-Index: AQHWcP/N3M3Ik/Umo0KIaIZ27EFWNw==
Date:   Thu, 13 Aug 2020 02:38:20 +0000
Message-ID: <BYAPR04MB496599E8B5ECCEBEE3E461CE86430@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <20200812232444.18310-1-logang@deltatee.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 90cba969-9ce8-4c95-360a-08d83f31f151
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Aug 2020 02:38:20.6214
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1Bq3Fxvph4ERMIxQ4eF6KN7RTwp/SaB/FnUHiAyABbTjxe6BZO3QYn6acHiqL7wJt6lvIUJKlaDIfLGJvgMRQRIN3ozJv41/t9UukSa8MoI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4871
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/12/20 16:25, Logan Gunthorpe wrote:=0A=
> When locking the ctrl->lock spinlock IRQs need to be disabled to avoid a=
=0A=
> dead lock. The new spin_lock() calls recently added produce the=0A=
> following lockdep warning when running the blktest nvme/003:=0A=
> =0A=
>      =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
>      WARNING: inconsistent lock state=0A=
>      --------------------------------=0A=
>      inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.=0A=
>      ksoftirqd/2/22 [HC0[0]:SC1[1]:HE0:SE0] takes:=0A=
>      ffff888276a8c4c0 (&ctrl->lock){+.?.}-{2:2}, at: nvme_keep_alive_end_=
io+0x50/0xc0=0A=
>      {SOFTIRQ-ON-W} state was registered at:=0A=
>        lock_acquire+0x164/0x500=0A=
>        _raw_spin_lock+0x28/0x40=0A=
>        nvme_get_effects_log+0x37/0x1c0=0A=
>        nvme_init_identify+0x9e4/0x14f0=0A=
>        nvme_reset_work+0xadd/0x2360=0A=
>        process_one_work+0x66b/0xb70=0A=
>        worker_thread+0x6e/0x6c0=0A=
>        kthread+0x1e7/0x210=0A=
>        ret_from_fork+0x22/0x30=0A=
>      irq event stamp: 1449221=0A=
>      hardirqs last  enabled at (1449220): [<ffffffff81c58e69>] ktime_get+=
0xf9/0x140=0A=
>      hardirqs last disabled at (1449221): [<ffffffff83129665>] _raw_spin_=
lock_irqsave+0x25/0x60=0A=
>      softirqs last  enabled at (1449210): [<ffffffff83400447>] __do_softi=
rq+0x447/0x595=0A=
>      softirqs last disabled at (1449215): [<ffffffff81b489b5>] run_ksofti=
rqd+0x35/0x50=0A=
> =0A=
>      other info that might help us debug this:=0A=
>       Possible unsafe locking scenario:=0A=
> =0A=
>             CPU0=0A=
>             ----=0A=
>        lock(&ctrl->lock);=0A=
>        <Interrupt>=0A=
>          lock(&ctrl->lock);=0A=
> =0A=
>       *** DEADLOCK ***=0A=
> =0A=
>      no locks held by ksoftirqd/2/22.=0A=
> =0A=
>      stack backtrace:=0A=
>      CPU: 2 PID: 22 Comm: ksoftirqd/2 Not tainted 5.8.0-rc4-eid-vmlocalye=
s-dbg-00157-g7236657c6b3a #1450=0A=
>      Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04=
/01/2014=0A=
>      Call Trace:=0A=
>       dump_stack+0xc8/0x11a=0A=
>       print_usage_bug.cold.63+0x235/0x23e=0A=
>       mark_lock+0xa9c/0xcf0=0A=
>       __lock_acquire+0xd9a/0x2b50=0A=
>       lock_acquire+0x164/0x500=0A=
>       _raw_spin_lock_irqsave+0x40/0x60=0A=
>       nvme_keep_alive_end_io+0x50/0xc0=0A=
>       blk_mq_end_request+0x158/0x210=0A=
>       nvme_complete_rq+0x146/0x500=0A=
>       nvme_loop_complete_rq+0x26/0x30 [nvme_loop]=0A=
>       blk_done_softirq+0x187/0x1e0=0A=
>       __do_softirq+0x118/0x595=0A=
>       run_ksoftirqd+0x35/0x50=0A=
>       smpboot_thread_fn+0x1d3/0x310=0A=
>       kthread+0x1e7/0x210=0A=
>       ret_from_fork+0x22/0x30=0A=
> =0A=
> Fixes: be93e87e7802 ("nvme: support for multiple Command Sets Supported a=
nd Effects log pages")=0A=
> Signed-off-by: Logan Gunthorpe<logang@deltatee.com>=0A=
=0A=
Thanks for this fix, looks good.=0A=
=0A=
Tested-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>=0A=
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>=0A=