From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752388AbdEXLHt (ORCPT <rfc822;w@1wt.eu>);
        Wed, 24 May 2017 07:07:49 -0400
Received: from mx2.mpynet.fi ([82.197.21.85]:10395 "EHLO mx2.mpynet.fi"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S969427AbdEXKZ2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 24 May 2017 06:25:28 -0400
Date: Wed, 24 May 2017 13:25:00 +0300
From: Rakesh Pandit <rakesh@tuxera.com>
To: Christoph Hellwig <hch@lst.de>
CC: <linux-nvme@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
        Keith Busch <keith.busch@intel.com>, Jens Axboe <axboe@fb.com>,
        Sagi Grimberg <sagi@grimberg.me>
Subject: Re: [PATCH 1/2] nvme: fix multiple ctrl removal scheduling
Message-ID: <20170524102500.GA26375@dhcp-216.srv.tuxera.com>
References: <20170523221545.GA23946@dhcp-216.srv.tuxera.com>
 <20170524093755.GA17483@lst.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20170524093755.GA17483@lst.de>
User-Agent: Mutt/1.7.1 (2016-10-04)
X-ClientProxiedBy: tuxera-exch.ad.tuxera.com (10.20.48.11) To
 tuxera-exch.ad.tuxera.com (10.20.48.11)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, May 24, 2017 at 11:37:55AM +0200, Christoph Hellwig wrote:
> On Wed, May 24, 2017 at 01:15:47AM +0300, Rakesh Pandit wrote:
> > Commit c5f6ce97c1210 tries to address multiple resets but fails as
> > work_busy doesn't involve any synchronization and can fail.  This is
> > reproducible easily as can be seen by WARNING below which is triggered
> > with line:
> > 
> > WARN_ON(dev->ctrl.state == NVME_CTRL_RESETTING)
> > 
> > Allowing multiple resets can result in multiple controller removal as
> > well if different conditions inside nvme_reset_work fail and which
> > might deadlock on device_release_driver.
> > 
> > This patch addresses the problem by using state of controller to
> > decide whether reset should be queued or not as state change is
> > synchronizated using controller spinlock.
> 
> But we don't hold the lock over the check and the decision.  I suspect

Thanks, right.

> what we need to do is to actually change to the resetting state
> before queueing up the reset work.  Can you give that a spin?

Sure, will post it in next version.