qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: Matias Bjorling <Matias.Bjorling@wdc.com>
Cc: "Fam Zheng" <fam@euphon.net>, "Kevin Wolf" <kwolf@redhat.com>,
	"Damien Le Moal" <Damien.LeMoal@wdc.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	"Niklas Cassel" <Niklas.Cassel@wdc.com>,
	"Klaus Jensen" <k.jensen@samsung.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Alistair Francis" <Alistair.Francis@wdc.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH v4 00/14] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set
Date: Tue, 29 Sep 2020 20:36:21 +0200	[thread overview]
Message-ID: <20200929183621.GE286786@apples.localdomain> (raw)
In-Reply-To: <MN2PR04MB62232F2E33E3FCC880E47472F1320@MN2PR04MB6223.namprd04.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 4677 bytes --]

On Sep 29 18:17, Matias Bjorling wrote:
> 
> 
> > -----Original Message-----
> > From: Klaus Jensen <its@irrelevant.dk>
> > Sent: Tuesday, 29 September 2020 20.00
> > To: Keith Busch <kbusch@kernel.org>
> > Cc: Damien Le Moal <Damien.LeMoal@wdc.com>; Fam Zheng
> > <fam@euphon.net>; Kevin Wolf <kwolf@redhat.com>; qemu-
> > block@nongnu.org; Niklas Cassel <Niklas.Cassel@wdc.com>; Klaus Jensen
> > <k.jensen@samsung.com>; qemu-devel@nongnu.org; Alistair Francis
> > <Alistair.Francis@wdc.com>; Philippe Mathieu-Daudé <philmd@redhat.com>;
> > Matias Bjorling <Matias.Bjorling@wdc.com>
> > Subject: Re: [PATCH v4 00/14] hw/block/nvme: Support Namespace Types and
> > Zoned Namespace Command Set
> > 
> > On Sep 29 10:29, Keith Busch wrote:
> > > On Tue, Sep 29, 2020 at 12:46:33PM +0200, Klaus Jensen wrote:
> > > > It is unmistakably clear that you are invalidating my arguments
> > > > about portability and endianness issues by suggesting that we just
> > > > remove persistent state and deal with it later, but persistence is
> > > > the killer feature that sets the QEMU emulated device apart from
> > > > other emulation options. It is not about using emulation in
> > > > production (because yeah, why would you?), but persistence is what
> > > > makes it possible to develop and test "zoned FTLs" or something that
> > requires recovery at power up.
> > > > This is what allows testing of how your host software deals with
> > > > opened zones being transitioned to FULL on power up and the
> > > > persistent tracking of LBA allocation (in my series) can be used to
> > > > properly test error recovery if you lost state in the app.
> > >
> > > Hold up -- why does an OPEN zone transition to FULL on power up? The
> > > spec suggests it should be CLOSED. The spec does appear to support
> > > going to FULL on a NVM Subsystem Reset, though. Actually, now that I'm
> > > looking at this part of the spec, these implicit transitions seem a
> > > bit less clear than I expected. I'm not sure it's clear enough to
> > > evaluate qemu's compliance right now.
> > >
> > > But I don't see what testing these transitions has to do with having a
> > > persistent state. You can reboot your VM without tearing down the
> > > running QEMU instance. You can also unbind the driver or shutdown the
> > > controller within the running operating system. That should make those
> > > implicit state transitions reachable in order to exercise your FTL's
> > > recovery.
> > >
> > 
> > Oh dear - don't "spec" with me ;)
> > 
> > NVMe v1.4 Section 7.3.1:
> > 
> >     An NVM Subsystem Reset is initiated when:
> >       * Main power is applied to the NVM subsystem;
> >       * A value of 4E564D64h ("NVMe") is written to the NSSR.NSSRC
> >         field;
> >       * Requested using a method defined in the NVMe Management
> >         Interface specification; or
> >       * A vendor specific event occurs.
> > 
> > In the context of QEMU, "Main power" is tearing down QEMU and starting it
> > from scratch. Just like on a "real" host, unbinding the driver, rebooting or
> > shutting down the controller does not cause a subsystem reset (and does not
> > cause the zones to change state). And since the device does not indicate
> > support for the optional NSSR.NSSRC register, that way to initiate a subsystem
> > cannot be used.
> > 
> > The reason for moving to FULL is that write pointer updates are not persisted
> > on each advancement, only when the zone state changes. So zones that were
> > opened might have valid data, but invalid write pointer.
> > So the device transitions them to FULL as it is allowed to.
> > 
> 
> How about when one must also recover from intermediate states (i.e.,
> open/closed upon power loss). For example, I don't hope a real SSD
> implementation transition zones to full when it has thousands of open
> simultaneously. That could be a disaster for the PE cycles, and a lot
> of media going to waste. One would want applications to support that
> kind of failure mode as well. 

Christ. The WDC Strike Force is really jumping out of lightspeed here.
I'm afraid I don't have an opposing force to engage with. So I'll be
your only boxing bag for the evening.

As Keith just said, "Opened" is not a valid intial state. Didn't you
write the spec? ;) As for Closed, they will be brought up as is.

With that in mind, I'm not sure what you specifically refer to? I'll
gently remind you that the QEMU nvme device is not a real SSD and does
not deal with NAND so it does not really do any "recovering" of
intermediate states on power on if that is what you refer to?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-09-29 18:41 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23 18:20 [PATCH v4 00/14] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 01/14] hw/block/nvme: Report actual LBA data shift in LBAF Dmitry Fomichev
2020-09-24 12:12   ` Klaus Jensen
2020-09-23 18:20 ` [PATCH v4 02/14] hw/block/nvme: Add Commands Supported and Effects log Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 03/14] hw/block/nvme: Introduce the Namespace Types definitions Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 04/14] hw/block/nvme: Define trace events related to NS Types Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 05/14] hw/block/nvme: Add support for Namespace Types Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 06/14] hw/block/nvme: Add support for active/inactive namespaces Dmitry Fomichev
2020-09-24 12:12   ` Klaus Jensen
2020-09-24 18:17     ` Niklas Cassel
2020-09-24 18:55       ` Klaus Jensen
2020-09-24 19:40         ` Niklas Cassel
2020-09-23 18:20 ` [PATCH v4 07/14] hw/block/nvme: Make Zoned NS Command Set definitions Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 08/14] hw/block/nvme: Define Zoned NS Command Set trace events Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 09/14] hw/block/nvme: Support Zoned Namespace Command Set Dmitry Fomichev
2020-09-25 18:24   ` Klaus Jensen
2020-09-23 18:20 ` [PATCH v4 10/14] hw/block/nvme: Introduce max active and open zone limits Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 11/14] hw/block/nvme: Support Zone Descriptor Extensions Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 12/14] hw/block/nvme: Add injection of Offline/Read-Only zones Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 13/14] hw/block/nvme: Use zone metadata file for persistence Dmitry Fomichev
2020-09-23 18:20 ` [PATCH v4 14/14] hw/block/nvme: Document zoned parameters in usage text Dmitry Fomichev
2020-09-24 21:07 ` [PATCH v4 00/14] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set Klaus Jensen
2020-09-28  2:33   ` Dmitry Fomichev
2020-09-28  6:36     ` Klaus Jensen
2020-09-28 21:25       ` Keith Busch
2020-09-28 22:54         ` Damien Le Moal
2020-09-29 10:46           ` Klaus Jensen
2020-09-29 11:13             ` Damien Le Moal
2020-09-29 17:44               ` Keith Busch
2020-09-29 15:43             ` Dmitry Fomichev
2020-09-29 16:36               ` Klaus Jensen
2020-09-29 17:29             ` Keith Busch
2020-09-29 18:00               ` Klaus Jensen
2020-09-29 18:15                 ` Keith Busch
2020-09-29 18:18                   ` Klaus Jensen
2020-09-29 18:17                 ` Matias Bjorling
2020-09-29 18:36                   ` Klaus Jensen [this message]
2020-09-29 19:42                     ` Matias Bjorling
2020-09-29 15:42       ` Dmitry Fomichev
2020-09-29 18:39         ` Klaus Jensen
2020-09-29 19:22           ` Keith Busch
2020-09-29 19:53             ` Dmitry Fomichev

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=20200929183621.GE286786@apples.localdomain \
    --to=its@irrelevant.dk \
    --cc=Alistair.Francis@wdc.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=Matias.Bjorling@wdc.com \
    --cc=Niklas.Cassel@wdc.com \
    --cc=fam@euphon.net \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.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).