Message ID | 1600255098-21411-1-git-send-email-yoshihiro.shimoda.uh@renesas.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | [RFC] ata: sata-rcar: Fix .dma_boundary for WRITE DMA EXT timeout issue | expand |
Hi Shimoda-san, On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > When we wrote data to an SATA HDD, the following timeout issue > happened after the commit 429120f3df2d ("block: fix splitting > segments on boundary masks") was applied: > > # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 > ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen > ata1.00: failed command: WRITE DMA EXT > ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out > res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > ata1.00: status: { DRDY } > > Since the commit changed get_max_segment_size()'s behavior, > unexpected behavior happens if .dma_boundary of this sata-rcar driver > is 0x1ffffffe in somewhere (my guess). > > By the way, the commit 8bfbeed58665 ("sata_rcar: correct > 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this > number is related to ATAPI_DMA_TRANS_CNT register. So, we should set > the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set > .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). > > After applied this, the timeout issue disappeared. > > Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- > As I wrote the commit description, I couldn't find why the issue > was related to the .dma_boundary. So, I marked RFC on this patch. > I would appreciate it if you would give me some advice. There's also "[PATCH v2] ata: sata_rcar: Fix DMA boundary mask" (https://lore.kernel.org/linux-ide/20200811081712.4981-1-geert+renesas@glider.be/) Is this related? Does my patch fix your issue, too? > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c > @@ -120,7 +120,7 @@ > /* Descriptor table word 0 bit (when DTA32M = 1) */ > #define SATA_RCAR_DTEND BIT(0) > > -#define SATA_RCAR_DMA_BOUNDARY 0x1FFFFFFEUL > +#define SATA_RCAR_DMA_TRANS_CNT 0x1FFFFFFEUL > > /* Gen2 Physical Layer Control Registers */ > #define RCAR_GEN2_PHY_CTL1_REG 0x1704 > @@ -636,14 +636,7 @@ static u8 sata_rcar_bmdma_status(struct ata_port *ap) > } > > static struct scsi_host_template sata_rcar_sht = { > - ATA_BASE_SHT(DRV_NAME), > - /* > - * This controller allows transfer chunks up to 512MB which cross 64KB > - * boundaries, therefore the DMA limits are more relaxed than standard > - * ATA SFF. > - */ > - .sg_tablesize = ATA_MAX_PRD, > - .dma_boundary = SATA_RCAR_DMA_BOUNDARY, > + ATA_BMDMA_SHT(DRV_NAME), > }; > > static struct ata_port_operations sata_rcar_port_ops = { > @@ -930,6 +923,14 @@ static int sata_rcar_probe(struct platform_device *pdev) > /* initialize host controller */ > sata_rcar_init_controller(host); > > + /* > + * This controller allows transfer chunks up to 512MB which cross 64KB > + * boundaries, therefore the DMA limits are more relaxed than standard > + * ATA SFF. > + */ > + sata_rcar_sht.max_segment_size = min_t(unsigned int, > + SATA_RCAR_DMA_TRANS_CNT, > + dma_max_mapping_size(dev)); > ret = ata_host_activate(host, irq, sata_rcar_interrupt, 0, > &sata_rcar_sht); > if (!ret) Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Wednesday, September 16, 2020 8:48 PM > > Hi Shimoda-san, > > On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > When we wrote data to an SATA HDD, the following timeout issue > > happened after the commit 429120f3df2d ("block: fix splitting > > segments on boundary masks") was applied: > > > > # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 > > ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen > > ata1.00: failed command: WRITE DMA EXT > > ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out > > res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > > ata1.00: status: { DRDY } > > > > Since the commit changed get_max_segment_size()'s behavior, > > unexpected behavior happens if .dma_boundary of this sata-rcar driver > > is 0x1ffffffe in somewhere (my guess). > > > > By the way, the commit 8bfbeed58665 ("sata_rcar: correct > > 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this > > number is related to ATAPI_DMA_TRANS_CNT register. So, we should set > > the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set > > .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). > > > > After applied this, the timeout issue disappeared. > > > > Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! > > > --- > > As I wrote the commit description, I couldn't find why the issue > > was related to the .dma_boundary. So, I marked RFC on this patch. > > I would appreciate it if you would give me some advice. > > There's also "[PATCH v2] ata: sata_rcar: Fix DMA boundary mask" > (https://lore.kernel.org/linux-ide/20200811081712.4981-1-geert+renesas@glider.be/) > > Is this related? > Does my patch fix your issue, too? Thank you for the information! Your patch fixed my issue too. So, I think my patch should be dropped. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Thu, Sep 17, 2020 at 10:00 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, September 16, 2020 8:48 PM > > On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > When we wrote data to an SATA HDD, the following timeout issue > > > happened after the commit 429120f3df2d ("block: fix splitting > > > segments on boundary masks") was applied: > > > > > > # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 > > > ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen > > > ata1.00: failed command: WRITE DMA EXT > > > ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out > > > res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > > > ata1.00: status: { DRDY } > > > > > > Since the commit changed get_max_segment_size()'s behavior, > > > unexpected behavior happens if .dma_boundary of this sata-rcar driver > > > is 0x1ffffffe in somewhere (my guess). > > > > > > By the way, the commit 8bfbeed58665 ("sata_rcar: correct > > > 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this > > > number is related to ATAPI_DMA_TRANS_CNT register. So, we should set > > > the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set > > > .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). > > > > > > After applied this, the timeout issue disappeared. > > > > > > Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > Thanks for your patch! > > > > > --- > > > As I wrote the commit description, I couldn't find why the issue > > > was related to the .dma_boundary. So, I marked RFC on this patch. > > > I would appreciate it if you would give me some advice. > > > > There's also "[PATCH v2] ata: sata_rcar: Fix DMA boundary mask" > > (https://lore.kernel.org/linux-ide/20200811081712.4981-1-geert+renesas@glider.be/) > > > > Is this related? > > Does my patch fix your issue, too? > > Thank you for the information! > Your patch fixed my issue too. So, I think my patch should be dropped. Thanks for testing! Can I add your Tested-by? Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, September 17, 2020 5:05 PM > > Hi Shimoda-san, > > On Thu, Sep 17, 2020 at 10:00 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Wednesday, September 16, 2020 8:48 PM > > > On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > When we wrote data to an SATA HDD, the following timeout issue > > > > happened after the commit 429120f3df2d ("block: fix splitting > > > > segments on boundary masks") was applied: > > > > > > > > # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 > > > > ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen > > > > ata1.00: failed command: WRITE DMA EXT > > > > ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out > > > > res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > > > > ata1.00: status: { DRDY } > > > > > > > > Since the commit changed get_max_segment_size()'s behavior, > > > > unexpected behavior happens if .dma_boundary of this sata-rcar driver > > > > is 0x1ffffffe in somewhere (my guess). > > > > > > > > By the way, the commit 8bfbeed58665 ("sata_rcar: correct > > > > 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this > > > > number is related to ATAPI_DMA_TRANS_CNT register. So, we should set > > > > the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set > > > > .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). > > > > > > > > After applied this, the timeout issue disappeared. > > > > > > > > Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- > > > > As I wrote the commit description, I couldn't find why the issue > > > > was related to the .dma_boundary. So, I marked RFC on this patch. > > > > I would appreciate it if you would give me some advice. > > > > > > There's also "[PATCH v2] ata: sata_rcar: Fix DMA boundary mask" > > > (https://lore.kernel.org/linux-ide/20200811081712.4981-1-geert+renesas@glider.be/) > > > > > > Is this related? > > > Does my patch fix your issue, too? > > > > Thank you for the information! > > Your patch fixed my issue too. So, I think my patch should be dropped. > > Thanks for testing! > > Can I add your Tested-by? Yes. Thanks! Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Thu, Sep 17, 2020 at 10:14 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Thursday, September 17, 2020 5:05 PM > > On Thu, Sep 17, 2020 at 10:00 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > From: Geert Uytterhoeven, Sent: Wednesday, September 16, 2020 8:48 PM > > > > On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > When we wrote data to an SATA HDD, the following timeout issue > > > > > happened after the commit 429120f3df2d ("block: fix splitting > > > > > segments on boundary masks") was applied: > > > > > > > > > > # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 > > > > > ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen > > > > > ata1.00: failed command: WRITE DMA EXT > > > > > ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out > > > > > res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) > > > > > ata1.00: status: { DRDY } > > > > > > > > > > Since the commit changed get_max_segment_size()'s behavior, > > > > > unexpected behavior happens if .dma_boundary of this sata-rcar driver > > > > > is 0x1ffffffe in somewhere (my guess). > > > > > > > > > > By the way, the commit 8bfbeed58665 ("sata_rcar: correct > > > > > 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this > > > > > number is related to ATAPI_DMA_TRANS_CNT register. So, we should set > > > > > the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set > > > > > .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). > > > > > > > > > > After applied this, the timeout issue disappeared. > > > > > > > > > > Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- > > > > > As I wrote the commit description, I couldn't find why the issue > > > > > was related to the .dma_boundary. So, I marked RFC on this patch. > > > > > I would appreciate it if you would give me some advice. > > > > > > > > There's also "[PATCH v2] ata: sata_rcar: Fix DMA boundary mask" > > > > (https://lore.kernel.org/linux-ide/20200811081712.4981-1-geert+renesas@glider.be/) > > > > > > > > Is this related? > > > > Does my patch fix your issue, too? > > > > > > Thank you for the information! > > > Your patch fixed my issue too. So, I think my patch should be dropped. > > > > Thanks for testing! > > > > Can I add your Tested-by? > > Yes. Thanks! Thanks, I'll augment the patch description with the gist of your problem report. Gr{oetje,eeting}s, Geert
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index 141ac60..abd0752 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -120,7 +120,7 @@ /* Descriptor table word 0 bit (when DTA32M = 1) */ #define SATA_RCAR_DTEND BIT(0) -#define SATA_RCAR_DMA_BOUNDARY 0x1FFFFFFEUL +#define SATA_RCAR_DMA_TRANS_CNT 0x1FFFFFFEUL /* Gen2 Physical Layer Control Registers */ #define RCAR_GEN2_PHY_CTL1_REG 0x1704 @@ -636,14 +636,7 @@ static u8 sata_rcar_bmdma_status(struct ata_port *ap) } static struct scsi_host_template sata_rcar_sht = { - ATA_BASE_SHT(DRV_NAME), - /* - * This controller allows transfer chunks up to 512MB which cross 64KB - * boundaries, therefore the DMA limits are more relaxed than standard - * ATA SFF. - */ - .sg_tablesize = ATA_MAX_PRD, - .dma_boundary = SATA_RCAR_DMA_BOUNDARY, + ATA_BMDMA_SHT(DRV_NAME), }; static struct ata_port_operations sata_rcar_port_ops = { @@ -930,6 +923,14 @@ static int sata_rcar_probe(struct platform_device *pdev) /* initialize host controller */ sata_rcar_init_controller(host); + /* + * This controller allows transfer chunks up to 512MB which cross 64KB + * boundaries, therefore the DMA limits are more relaxed than standard + * ATA SFF. + */ + sata_rcar_sht.max_segment_size = min_t(unsigned int, + SATA_RCAR_DMA_TRANS_CNT, + dma_max_mapping_size(dev)); ret = ata_host_activate(host, irq, sata_rcar_interrupt, 0, &sata_rcar_sht); if (!ret)
When we wrote data to an SATA HDD, the following timeout issue happened after the commit 429120f3df2d ("block: fix splitting segments on boundary masks") was applied: # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024 ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen ata1.00: failed command: WRITE DMA EXT ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) ata1.00: status: { DRDY } Since the commit changed get_max_segment_size()'s behavior, unexpected behavior happens if .dma_boundary of this sata-rcar driver is 0x1ffffffe in somewhere (my guess). By the way, the commit 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") changed the .dma_boundary as 0x1ffffffe, but this number is related to ATAPI_DMA_TRANS_CNT register. So, we should set the .dma_boundary as ATA_DMA_BOUNDARY (0xffff), and set .max_segment_size to min(0x1ffffffe, dma_max_mapping_size()). After applied this, the timeout issue disappeared. Fixes: 8bfbeed58665 ("sata_rcar: correct 'sata_rcar_sht'") Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- As I wrote the commit description, I couldn't find why the issue was related to the .dma_boundary. So, I marked RFC on this patch. I would appreciate it if you would give me some advice. drivers/ata/sata_rcar.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)