RE: [PATCHv2 5/6] nvme: Prevent resets during paused states

From: Judy Brock <judy.brock@samsung.com>
To: Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>
Cc: Judy Brock <judy.brock@samsung.com>,
	Edmund Nadolski <edmund.nadolski@intel.com>,
	Sagi Grimberg <sagi@grimberg.me>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	James Smart <james.smart@broadcom.com>
Subject: RE: [PATCHv2 5/6] nvme: Prevent resets during paused states
Date: Thu, 10 Oct 2019 12:03:16 +0000	[thread overview]
Message-ID: <2f876efff1484f5f95b852584b27efc6@samsung.com> (raw)
In-Reply-To: <20190928002349.GA17232@C02WT3WMHTD6>

Hi Keith,

Any chance you can get this patch in to the nvme tree soon? 

We believe it may fix an issue we have seen. We are throwing a ton of stuff at two controllers on a dual ported device at the same time including multiple nested resets, nested FW Activations, nested Formats, DSMs, FW Downloads, etc. We saw this in dmesg in one test run. We think Processing Paused bit was probably not valid at the time as we don't have any record of any FW Activates that went to either of the controllers and never finished.

kern  :warn  : [Wed Oct  2 14:21:06 2019] nvme nvme0: Fw activation timeout, reset controller
kern  :warn  : [Wed Oct  2 14:21:06 2019] nvme nvme1: Fw activation timeout, reset controller

thanks,

Judy

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of Keith Busch
Sent: Friday, September 27, 2019 5:24 PM
To: Christoph Hellwig
Cc: Edmund Nadolski; Sagi Grimberg; linux-nvme@lists.infradead.org; James Smart
Subject: Re: [PATCHv2 5/6] nvme: Prevent resets during paused states

On Fri, Sep 27, 2019 at 11:41:21PM +0200, Christoph Hellwig wrote:
> On Fri, Sep 20, 2019 at 06:34:30AM +0900, kbusch@kernel.org wrote:
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 95b74d020506..33b5729763c2 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -3742,15 +3742,14 @@ static void nvme_fw_act_work(struct work_struct *work)
> >  		if (time_after(jiffies, fw_act_timeout)) {
> >  			dev_warn(ctrl->device,
> >  				"Fw activation timeout, reset controller\n");
> > -			nvme_reset_ctrl(ctrl);
> > -			break;
> > +			nvme_reset_continue(ctrl);
> > +			return;
> 
> Hmm, I find the use of a function named nvme_reset_continue here
> rather confusing, as there is no reset we are continuing.  Yes, we place
> the controller in resetting state, but it isn't really a reset in the

A PAUSED state would have to copy the same transitions that RESETTING has. It
seems simpler to just collapse this with the existing RESETTING state, and we
also don't need a special way to schedule reset_work from it.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_mailman_listinfo_linux-2Dnvme&d=DwICAg&c=JfeWlBa6VbDyTXraMENjy_b_0yKWuqQ4qY-FPhxK4x8w-TfgRBDyeV4hVQQBEgL2&r=YJM_QPk2w1CRIo5NNBXnCXGzNnmIIfG_iTRs6chBf6s&m=esS-yc3HRUgI7AHbyjPV-1pJ3qRobo7jEiiqE-oMMpE&s=cCuZq1RCdLq7SrMkSFBJiSVlT_GcXGTKTN7e_55QRwE&e= 

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme