Hi Shimoda-san,
On Wed, Sep 16, 2020 at 1:27 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> 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@xxxxxxxxxxx>
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@xxxxxxxxx/)
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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
