Re: [PATCH #upstream-fixes] libata: fix ATAPI draining

[Jeff Garzik <jeff@garzik.org>]

Tejun Heo wrote:
> With ATAPI transfer chunk size properly programmed, libata PIO HSM
> should be able to handle full spurious data chunks.  Also, it's a good
> idea to suppress trailing data warning for misc ATAPI commands as
> there can be many of them per command - for example, if the chunk size
> is 16 and the drive tries to transfer 510 bytes, there can be 31
> trailing data messages.
> 
> This patch makes the following updates to libata ATAPI PIO HSM
> implementation.
> 
> * Make it drain full spurious chunks.
> 
> * Suppress trailing data warning message for misc commands.
> 
> * Put limit on how many bytes can be drained.
> 
> * If odd, round up consumed bytes and the number of bytes to be
>   drained.  This gets the number of bytes to drain right for drivers
>   which do 16bit PIO.
> 
> This patch is partial backport of improve-ATAPI-data-xfer patchset
> pending for #upstream.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> This combined with the previous patch fixes bug 9346.
> 
>  drivers/ata/libata-core.c |   87 ++++++++++++++++++++++++++++++++++------------
>  include/linux/libata.h    |    2 +
>  2 files changed, 67 insertions(+), 22 deletions(-)
> 
> Index: work/drivers/ata/libata-core.c
> ===================================================================
> --- work.orig/drivers/ata/libata-core.c
> +++ work/drivers/ata/libata-core.c
> @@ -64,6 +64,7 @@
>  #include <linux/libata.h>
>  #include <asm/semaphore.h>
>  #include <asm/byteorder.h>
> +#include <linux/cdrom.h>
>  
>  #include "libata.h"
>  
> @@ -4649,6 +4650,43 @@ int ata_check_atapi_dma(struct ata_queue
>  }
>  
>  /**
> + *	atapi_qc_may_overflow - Check whether data transfer may overflow
> + *	@qc: ATA command in question
> + *
> + *	ATAPI commands which transfer variable length data to host
> + *	might overflow due to application error or hardare bug.  This
> + *	function checks whether overflow should be drained and ignored
> + *	for @qc.
> + *
> + *	LOCKING:
> + *	None.
> + *
> + *	RETURNS:
> + *	1 if @qc may overflow; otherwise, 0.
> + */
> +static int atapi_qc_may_overflow(struct ata_queued_cmd *qc)
> +{
> +	if (qc->tf.protocol != ATA_PROT_ATAPI &&
> +	    qc->tf.protocol != ATA_PROT_ATAPI_DMA)
> +		return 0;
> +
> +	if (qc->tf.flags & ATA_TFLAG_WRITE)
> +		return 0;
> +
> +	switch (qc->cdb[0]) {
> +	case READ_10:
> +	case READ_12:
> +	case WRITE_10:
> +	case WRITE_12:
> +	case GPCMD_READ_CD:
> +	case GPCMD_READ_CD_MSF:
> +		return 0;
> +	}
> +
> +	return 1;

applied, though I hope we can eventually find a better solution...