| Message ID | 1430790861-30066-4-git-send-email-martin.petersen@oracle.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On 05/05/2015 03:54 AM, Martin K. Petersen wrote: > Some devices advertise support for the READ/WRITE LOG DMA EXT commands > but fail when we try to issue them. This can lead to queued TRIM being > unintentionally disabled since the relevant feature flag is located in a > general purpose log page. > > Fall back to unqueued READ LOG EXT if the DMA variant fails while > reading a log page. > > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > --- > drivers/ata/libata-eh.c | 12 +++++++++++- > include/linux/libata.h | 1 + > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c > index 07f41be38fbe..2893563d0537 100644 > --- a/drivers/ata/libata-eh.c > +++ b/drivers/ata/libata-eh.c > @@ -1507,13 +1507,17 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, > { > struct ata_taskfile tf; > unsigned int err_mask; > + bool dma = false; > > DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page); > > +retry: > ata_tf_init(dev, &tf); > - if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id)) { > + if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) && > + !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) { > tf.command = ATA_CMD_READ_LOG_DMA_EXT; > tf.protocol = ATA_PROT_DMA; > + dma = true; > } else { > tf.command = ATA_CMD_READ_LOG_EXT; > tf.protocol = ATA_PROT_PIO; > @@ -1527,6 +1531,12 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, > err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, > buf, sectors * ATA_SECT_SIZE, 0); > > + if (err_mask && dma) { > + dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG; > + ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n"); > + goto retry; > + } > + > DPRINTK("EXIT, err_mask=%x\n", err_mask); > return err_mask; > } > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 8dad4a307bb8..c3ef58014b33 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -424,6 +424,7 @@ enum { > ATA_HORKAGE_NOLPM = (1 << 20), /* don't use LPM */ > ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21), /* some WDs have broken LPM */ > ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */ > + ATA_HORKAGE_NO_NCQ_LOG = (1 << 23), /* don't use NCQ for log read */ > > /* DMA mask for user DMA control: User visible values; DO NOT > renumber */ > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Hi Martin, On 05/04/2015 08:54 PM, Martin K. Petersen wrote: > Some devices advertise support for the READ/WRITE LOG DMA EXT commands > but fail when we try to issue them. This can lead to queued TRIM being > unintentionally disabled since the relevant feature flag is located in a > general purpose log page. > > Fall back to unqueued READ LOG EXT if the DMA variant fails while > reading a log page. Have you seen both commands fail on any of the Samsung SSD 840 EVO drives? # dmesg | grep Samsung [ 2.254443] ata10.00: ATA-9: Samsung SSD 840 EVO 1TB, EXT0BB6Q, max UDMA/133 <dmesg output> [ 2.254390] ata10.00: READ LOG DMA EXT failed, trying unqueued [ 2.254441] ata10.00: failed to get NCQ Send/Recv Log Emask 0x1 <dmesg output> (In this case it also failed READ LOG EXT) This is linux-4.1.0-rc7 patched with the following from http://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.2 libata: Allow NCQ TRIM to be enabled or disabled with a module parameter libata: Expose TRIM capability in sysfs libata: READ LOG DMA EXT support can be in either page 119 or 120 libata: Fall back to unqueued READ LOG EXT if the DMA variant fails libata: Fix regression when the NCQ Send and Receive log page is absent Thanks, David > > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > --- > drivers/ata/libata-eh.c | 12 +++++++++++- > include/linux/libata.h | 1 + > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c > index 07f41be38fbe..2893563d0537 100644 > --- a/drivers/ata/libata-eh.c > +++ b/drivers/ata/libata-eh.c > @@ -1507,13 +1507,17 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, > { > struct ata_taskfile tf; > unsigned int err_mask; > + bool dma = false; > > DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page); > > +retry: > ata_tf_init(dev, &tf); > - if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id)) { > + if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) && > + !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) { > tf.command = ATA_CMD_READ_LOG_DMA_EXT; > tf.protocol = ATA_PROT_DMA; > + dma = true; > } else { > tf.command = ATA_CMD_READ_LOG_EXT; > tf.protocol = ATA_PROT_PIO; > @@ -1527,6 +1531,12 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, > err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, > buf, sectors * ATA_SECT_SIZE, 0); > > + if (err_mask && dma) { > + dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG; > + ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n"); > + goto retry; > + } > + > DPRINTK("EXIT, err_mask=%x\n", err_mask); > return err_mask; > } > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 8dad4a307bb8..c3ef58014b33 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -424,6 +424,7 @@ enum { > ATA_HORKAGE_NOLPM = (1 << 20), /* don't use LPM */ > ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21), /* some WDs have broken LPM */ > ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */ > + ATA_HORKAGE_NO_NCQ_LOG = (1 << 23), /* don't use NCQ for log read */ > > /* DMA mask for user DMA control: User visible values; DO NOT > renumber */ > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "David" == David Milburn <dmilburn@redhat.com> writes:
David> Have you seen both commands fail on any of the Samsung SSD 840
David> EVO drives?
David> # dmesg | grep Samsung [ 2.254443] ata10.00: ATA-9: Samsung SSD
David> 840 EVO 1TB, EXT0BB6Q, max UDMA/133
I have an 840 EVO in my test box. However, yours is running an older
firmware that predates queued TRIM support. So I guess that's
expected...
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 07f41be38fbe..2893563d0537 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1507,13 +1507,17 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, { struct ata_taskfile tf; unsigned int err_mask; + bool dma = false; DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page); +retry: ata_tf_init(dev, &tf); - if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id)) { + if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) && + !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) { tf.command = ATA_CMD_READ_LOG_DMA_EXT; tf.protocol = ATA_PROT_DMA; + dma = true; } else { tf.command = ATA_CMD_READ_LOG_EXT; tf.protocol = ATA_PROT_PIO; @@ -1527,6 +1531,12 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log, err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, buf, sectors * ATA_SECT_SIZE, 0); + if (err_mask && dma) { + dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG; + ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n"); + goto retry; + } + DPRINTK("EXIT, err_mask=%x\n", err_mask); return err_mask; } diff --git a/include/linux/libata.h b/include/linux/libata.h index 8dad4a307bb8..c3ef58014b33 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -424,6 +424,7 @@ enum { ATA_HORKAGE_NOLPM = (1 << 20), /* don't use LPM */ ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21), /* some WDs have broken LPM */ ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */ + ATA_HORKAGE_NO_NCQ_LOG = (1 << 23), /* don't use NCQ for log read */ /* DMA mask for user DMA control: User visible values; DO NOT renumber */
Some devices advertise support for the READ/WRITE LOG DMA EXT commands but fail when we try to issue them. This can lead to queued TRIM being unintentionally disabled since the relevant feature flag is located in a general purpose log page. Fall back to unqueued READ LOG EXT if the DMA variant fails while reading a log page. Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> --- drivers/ata/libata-eh.c | 12 +++++++++++- include/linux/libata.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-)