Tejun Heo wrote:
> For misc ATAPI commands which transfer variable length data to the
> host, overflow can occur due to application or hardware bug. Such
> overflows can be ignored safely as long as overflow data is properly
> drained. libata HSM implementation has this implemented in
> __atapi_pio_bytes() but it isn't enough. Improve drain logic such
> that...
>
> * Multiple PIO data phases are allowed. Not allowing this used to be
> okay when transfer chunk size was set to 8k unconditionally but with
> transfer hcunk size set to allocation size, treating extra PIO data
> phases as HSM violations cause a lot of trouble.
>
> * Limit the amount of draining to ATAPI_MAX_DRAIN (16k currently).
>
> * Don't whine if overflow is allowed and safe. When unexpected
> overflow occurs, trigger HSM violation and report the problem using
> ehi error description.
>
> * Properly calculate the number of bytes to be drained considering
> actual number of consumed bytes for partial draining.
>
> * Add and use ata_drain_page for draining. This change fixes the
> problem where LLDs which do 32bit IOs consumes 4 bytes on each 2
> byte draining resulting in draining twice more data than requested.
>
> This patch fixes ATAPI regressions introduced by setting transfer
> chunk size to allocation size.
>
> Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
> Cc: Albert Lee <albertcc@xxxxxxxxxx>
Sorry for the late reply. The new patch looks good.
However, I am a little worried about the following code segment in
__atapi_pio_bytes():
next_sg:
if (!bytes)
return 0;
Maybe we should add an additional check to atapi_pio_bytes() such
that "DRQ asserted with bytes = 0" is considered as AC_ERR_HSM?
(patch attached below.) Otherwise the device might keep interruping us
with DRQ asserted + zero byte count, and we are in infinite loop...
--
albert
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c 07_more_check/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-11-14 10:08:36.000000000 +0800
+++ 07_more_check/drivers/ata/libata-core.c 2007-12-04 18:03:48.000000000 +0800
@@ -5341,6 +5341,9 @@ static void atapi_pio_bytes(struct ata_q
if (do_write != i_write)
goto err_out;
+ if (!bytes)
+ goto err_out;
+
VPRINTK("ata%u: xfering %d bytes\n", ap->print_id, bytes);
__atapi_pio_bytes(qc, bytes);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
