nvme-pci: Add Write Zero support to Intel 660p
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Given Intel 660p returns 0 when reading discarded data, allow Write Zero.
nvme id-ns /dev/nvme0n1
...
dlfeat : 1
...
Check Write Zeros is now enabled:
cat /sys/block/nvme0n1/queue/write_zeroes_max_bytes 2199023255040
--- drivers/nvme/host/pci.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/nvme/host/pci.c +2 -0
@@ -2694,6 +2694,8 @@ static const struct pci_device_id nvme_id_table[] = { { PCI_VDEVICE(INTEL, 0xf1a5), /* Intel 600P/P3100 */ .driver_data =NVME_QUIRK_NO_DEEPEST_PS | NVME_QUIRK_MEDIUM_PRIO_SQ }, + { PCI_VDEVICE(INTEL, 0xf1a8), /* Intel 660P */ + .driver_data = NVME_QUIRK_DEALLOCATE_ZEROES, }, { PCI_VDEVICE(INTEL, 0x5845), /* Qemu emulated controller */ .driver_data = NVME_QUIRK_IDENTIFY_CNS, }, { PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
Signature · 1 line
2.19.1.568.g152ad8e336-goog
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 22 lines · Gwendal Grignou
Given Intel 660p returns 0 when reading discarded data, allow Write Zero.
nvme id-ns
/dev/nvme0n1
...
dlfeat : 1
...Check Write Zeros is now enabled:
cat/sys/block/nvme0n1/queue/write_zeroes_max_bytes2199023255040Signed-off-by Gwendal Grignou <gwendal at chromium.org>--- drivers/nvme/host/pci.c | 2 ++ 1 file changed, 2 insertions(+)drivers/nvme/host/pci.c +0
-0
drivers/nvme/host/pci.c static const struct pci_device_id nvme_id_table[] = { · line 2698
Quoted 5 lines
drivers/nvme/host/pci.c +2
-0@@ -2694,6 +2694,8 @@ static const struct pci_device_id nvme_id_table[] = { { PCI_VDEVICE(INTEL, 0xf1a5), /* Intel 600P/P3100 */ .driver_data =NVME_QUIRK_NO_DEEPEST_PS | NVME_QUIRK_MEDIUM_PRIO_SQ }, + { PCI_VDEVICE(INTEL, 0xf1a8), /* Intel 660P */ + .driver_data = NVME_QUIRK_DEALLOCATE_ZEROES, },
The quirk is for devices that pre-date the ability to discover the capability. Now that we have a spec defined way to know a device's deallocate behavior, and this particular device implements that method, we should use that instead of a quirk.
Christoph Hellwig
maintainerPATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 4 lines · Keith Busch
The quirk is for devices that pre-date the ability to discover the capability. Now that we have a spec defined way to know a device's deallocate behavior, and this particular device implements that method, we should use that instead of a quirk.
The 'Deallocate Logical Block Features' bits 0 and 1 only tell what is read back from deallocated logic blocks, but it does not guarantee that all blocks a DSM deallocate is called on are actually deallocated.
That being said I really don't want to add more 'feature hacks' like this, it is time that we go back to a proper Write Zeroes implementation.
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
From: Linux-nvme <[email protected]> on behalf of Christoph Hellwig <[email protected]>
Sent: Saturday, October 27, 2018 12:44 AM
To: Keith Busch
Cc: axboe at fb.com; Gwendal Grignou; hch at lst.de; linux-nvme at lists.infradead.org; sagi at grimberg.me
Subject: Re: [PATCH] nvme-pci: Add Write Zero support to Intel 660p
?
Quoted 4 lines · Keith Busch
The quirk is for devices that pre-date the ability to discover the capability. Now that we have a spec defined way to know a device's deallocate behavior, and this particular device implements that method, we should use that instead of a quirk.
The 'Deallocate Logical Block Features' bits 0 and 1 only tell what is read back from deallocated logic blocks, but it does not guarantee that all blocks a DSM deallocate is called on are actually deallocated.
That being said I really don't want to add more 'feature hacks' like this, it is time that we go back to a proper Write Zeroes implementation.
[CK] I'll send out the patch shortly.
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
In reply to Christoph HellwigQuoted 9 lines · Christoph Hellwig
Quoted 4 lines · Keith Busch
The quirk is for devices that pre-date the ability to discover the capability. Now that we have a spec defined way to know a device's deallocate behavior, and this particular device implements that method, we should use that instead of a quirk.
The 'Deallocate Logical Block Features' bits 0 and 1 only tell what is read back from deallocated logic blocks, but it does not guarantee that all blocks a DSM deallocate is called on are actually deallocated.
Sure, but whether the requested blocks are deallocated or not is irrelevant. We're only using this feature here for its deterministic read behavior.
Quoted 2 lines
That being said I really don't want to add more 'feature hacks' like this, it is time that we go back to a proper Write Zeroes implementation.
Yes, I agree we ought to use nvme Write Zeroes instead of DSM if the controller supports it, but recall we had to revert the original implementation because it broke Linus' machine. The nvme part looked fine to me, though, so I don't know what the problem may have been.
Christoph Hellwig
maintainerPATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 7 lines · Keith Busch
Quoted 3 lines
The 'Deallocate Logical Block Features' bits 0 and 1 only tell what is read back from deallocated logic blocks, but it does not guarantee that all blocks a DSM deallocate is called on are actually deallocated.
Sure, but whether the requested blocks are deallocated or not is irrelevant. We're only using this feature here for its deterministic read behavior.
I can't parse this.
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 10 lines · Christoph Hellwig
Quoted 7 lines · Keith Busch
Quoted 3 lines
The 'Deallocate Logical Block Features' bits 0 and 1 only tell what is read back from deallocated logic blocks, but it does not guarantee that all blocks a DSM deallocate is called on are actually deallocated.
Sure, but whether the requested blocks are deallocated or not is irrelevant. We're only using this feature here for its deterministic read behavior.
I can't parse this.
We currently use DSM Deallocate for REQ_OP_WRITE_ZEROES. We don't care if the blocks are actually deallocated. We only care if reading them back determinisitically returns 0's, and DLFEAT tells us if we can rely on that behavior. I'm just suggesting to use that instead of using the NVME_QUIRK_DEALLOCATE_ZEROES quirk.
Christoph Hellwig
maintainerPATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 4 lines · Keith Busch
We currently use DSM Deallocate for
REQ_OP_WRITE_ZEROES. We don't care if the blocks are actually deallocated. We only care if reading them back determinisitically returns 0's, and DLFEAT tells us if we can rely on that behavior.
No, DLFEAT _only_ tells you the value you read from actually deallocated blocks. From the DLFEAT description:
"Bits 2:0 indicate the values read from a deallocated logical block and its metadata (excluding protection information). The values for this field have the following meanings:"
And from Section 6.7:
"This command is advisory; a compliant controller may choose to take no action based on information provided."
"Attribute ? Deallocate (AD): If set to ?1?, then the NVM subsystem may deallocate all provided ranges. The data returned for a deallocated range is specified in section 6.7.1.1."
PATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 13 lines · Christoph Hellwig
Quoted 4 lines · Keith Busch
We currently use DSM Deallocate for
REQ_OP_WRITE_ZEROES. We don't care if the blocks are actually deallocated. We only care if reading them back determinisitically returns 0's, and DLFEAT tells us if we can rely on that behavior.No, DLFEAT _only_ tells you the value you read from actually deallocated blocks. From the DLFEAT description:
"Bits 2:0 indicate the values read from a deallocated logical block and its metadata (excluding protection information). The values for this field have the following meanings:"
Quoted 8 lines
And from Section 6.7:
"This command is advisory; a compliant controller may choose to take no action based on information provided."
"Attribute ? Deallocate (AD): If set to ?1?, then the NVM subsystem may deallocate all provided ranges. The data returned for a deallocated range is specified in section 6.7.1.1."
Okay, that wording is unfortunate and the feature is useless if that's how it can be interpreted.
The "advisory" parts were originally directed toward the Context Attributes referring to access type and frequency.
The intended behavior for Deallocate was a controller picks exactly one behavior: return 0's, 1's, or the previous written data. It was never intended, as far as I know, for a controller to return read data differently for any given range after returning success to a DSM Deallocate command.
That must be at least part of the motivation for Write Zeroes adding deallocate support.
Christoph Hellwig
maintainerPATCH 1/1 nvme-pci: Add Write Zero support to Intel 660p
Quoted 2 lines · Keith Busch
Okay, that wording is unfortunate and the feature is useless if that's how it can be interpreted.
While I agree that it is unfortunate it matches how ATA TRIM and SCSI UNMAP work. And that is how it happened - I vague remember discussions in the working group.