[1/1] nvme-pci: disable write zeros support on WD SSDs

Message ID TYAPR03MB3085DD9F15177F126E9DF7C6AE249@TYAPR03MB3085.apcprd03.prod.outlook.com
State New
Headers
Series This patch disable write zeros support on WD SSDs. |

Commit Message

Ken Lin Oct. 14, 2022, 8:30 a.m. UTC
  Like commit 5611ec2b9814 ("nvme-pci: prevent SK hynix PC400 from using
Write Zeroes command"), Western Digital has the same issue:
[ 6305.633887] blk_update_request: operation not supported error,
dev nvme0n1, sector 340812032 op 0x9:(WRITE_ZEROES)
flags 0x0 phys_seg 0 prio class 0

So also disable Write Zeroes command on Western Digital.

Signed-off-by: Ken Lin <ken_lin5@hotmail.com>
---

 drivers/nvme/host/pci.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Doug Anderson Oct. 20, 2022, 5:15 p.m. UTC | #1
Hi,

On Fri, Oct 14, 2022 at 1:32 AM Ken Lin <ken_lin5@hotmail.com> wrote:
>
> Like commit 5611ec2b9814 ("nvme-pci: prevent SK hynix PC400 from using
> Write Zeroes command"), Western Digital has the same issue:
> [ 6305.633887] blk_update_request: operation not supported error,
> dev nvme0n1, sector 340812032 op 0x9:(WRITE_ZEROES)
> flags 0x0 phys_seg 0 prio class 0
>
> So also disable Write Zeroes command on Western Digital.
>
> Signed-off-by: Ken Lin <ken_lin5@hotmail.com>
> ---
>
>  drivers/nvme/host/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 3a1c37f32f30..5c1b812a3c2b 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3517,6 +3517,8 @@ static const struct pci_device_id nvme_id_table[] = {
>                 .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
>         { PCI_DEVICE(0xc0a9, 0x540a),   /* Crucial P2 */
>                 .driver_data = NVME_QUIRK_BOGUS_NID, },
> +       { PCI_DEVICE(0x15b7, 0x501e),   /* Sandisk 2280 NVMe SSD  */
> +               .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },

So I'm not really an expert and don't really have the context, but you
CCed me and so I took a quick glance.

Something smells a little fishy. ${SUBJECT} talks about disabling
"write zeros". The patch description talks about disabling "Write
Zeroes". The patch you reference seems to disable "write zeros" by
setting the quirk NVME_QUIRK_DISABLE_WRITE_ZEROES. ...but the contents
of your patch doesn't seem to match. Instead it adds the quirk
NVME_QUIRK_NO_DEEPEST_PS.

As I said, I'm not an expert and maybe everyone will tell me that it
all makes sense, but it seems weird.

-Doug
  
Keith Busch Oct. 20, 2022, 5:33 p.m. UTC | #2
On Thu, Oct 20, 2022 at 10:15:56AM -0700, Doug Anderson wrote:
> On Fri, Oct 14, 2022 at 1:32 AM Ken Lin <ken_lin5@hotmail.com> wrote:
> >
> > Like commit 5611ec2b9814 ("nvme-pci: prevent SK hynix PC400 from using
> > Write Zeroes command"), Western Digital has the same issue:
> > [ 6305.633887] blk_update_request: operation not supported error,
> > dev nvme0n1, sector 340812032 op 0x9:(WRITE_ZEROES)
> > flags 0x0 phys_seg 0 prio class 0
> >
> > So also disable Write Zeroes command on Western Digital.
> >
> > Signed-off-by: Ken Lin <ken_lin5@hotmail.com>
> > ---
> >
> >  drivers/nvme/host/pci.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > index 3a1c37f32f30..5c1b812a3c2b 100644
> > --- a/drivers/nvme/host/pci.c
> > +++ b/drivers/nvme/host/pci.c
> > @@ -3517,6 +3517,8 @@ static const struct pci_device_id nvme_id_table[] = {
> >                 .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
> >         { PCI_DEVICE(0xc0a9, 0x540a),   /* Crucial P2 */
> >                 .driver_data = NVME_QUIRK_BOGUS_NID, },
> > +       { PCI_DEVICE(0x15b7, 0x501e),   /* Sandisk 2280 NVMe SSD  */
> > +               .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
> 
> So I'm not really an expert and don't really have the context, but you
> CCed me and so I took a quick glance.
> 
> Something smells a little fishy. ${SUBJECT} talks about disabling
> "write zeros". The patch description talks about disabling "Write
> Zeroes". The patch you reference seems to disable "write zeros" by
> setting the quirk NVME_QUIRK_DISABLE_WRITE_ZEROES. ...but the contents
> of your patch doesn't seem to match. Instead it adds the quirk
> NVME_QUIRK_NO_DEEPEST_PS.
> 
> As I said, I'm not an expert and maybe everyone will tell me that it
> all makes sense, but it seems weird.

You're right: this patch does not make sense with the subject!
  

Patch

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 3a1c37f32f30..5c1b812a3c2b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3517,6 +3517,8 @@  static const struct pci_device_id nvme_id_table[] = {
 		.driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
 	{ PCI_DEVICE(0xc0a9, 0x540a),   /* Crucial P2 */
 		.driver_data = NVME_QUIRK_BOGUS_NID, },
+	{ PCI_DEVICE(0x15b7, 0x501e),   /* Sandisk 2280 NVMe SSD  */
+		.driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0061),
 		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0065),