Re: PATCH: (as141) Unaligned scatter-gather buffers and usb-storage

[Matthew Dharm <mdharm-scsi@one-eyed-alien.net>]

[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]

I'm going to hold on to this patch until I see the SCSI parts go in.  Alan,
try to remind me if you don't see this get merged after a while.

Matt

On Fri, Nov 21, 2003 at 01:03:05PM -0500, Alan Stern wrote:
> I'm not sure whether Jens is objecting to adding the extra member to the
> host template or to making the default alignment something other than 512.  
> Here's my attempt at a compromise patch, which boils the area of
> disagreement between Kai and Jens down to a single line of code (plus a
> comment).  It doesn't change the host template and it shouldn't slow down
> any tape I/O programs when using a non-USB device.
> 
> If there aren't any more objections to this, I urge James to apply the
> SCSI parts to a post-2.6.0-final tree, and likewise for Matt and the USB 
> part.
> 
> Alan Stern
> 
> 
> ===== scsiglue.c 1.60 vs edited =====
> --- 1.60/drivers/usb/storage/scsiglue.c	Fri Oct 24 14:53:38 2003
> +++ edited/drivers/usb/storage/scsiglue.c	Fri Nov 21 12:45:21 2003
> @@ -65,6 +65,16 @@
>  
>  static int slave_configure (struct scsi_device *sdev)
>  {
> +	/* Scatter-gather buffers (all but the last) must have a length
> +	 * divisible by the bulk maxpacket size.  Otherwise a data packet
> +	 * would end up being short, causing a premature end to the data
> +	 * transfer.  Since high-speed bulk pipes have a maxpacket size
> +	 * of 512, we'll use that as the scsi device queue's DMA alignment
> +	 * mask.  Guaranteeing proper alignment of the first buffer will
> +	 * have the desired effect because, except at the beginning and
> +	 * the end, scatter-gather buffers follow page boundaries. */
> +	blk_queue_dma_alignment(sdev->request_queue, (512 - 1));
> +
>  	/* this is to satisify the compiler, tho I don't think the 
>  	 * return code is ever checked anywhere. */
>  	return 0;
> ===== sg.c 1.48 vs edited =====
> --- 1.48/drivers/scsi/sg.c	Fri Oct 24 14:53:37 2003
> +++ edited/drivers/scsi/sg.c	Fri Nov 21 12:28:52 2003
> @@ -1741,7 +1741,11 @@
>  	int sg_tablesize = sfp->parentdp->sg_tablesize;
>  	struct scatterlist *sgl;
>  	int mx_sc_elems, res;
> +	struct scsi_device *sdev = sfp->parentdp->device;
>  
> +	if (((unsigned long)hp->dxferp &
> +			queue_dma_alignment(sdev->request_queue)) != 0)
> +		return 1;
>  	mx_sc_elems = sg_build_sgat(schp, sfp, sg_tablesize);
>          if (mx_sc_elems <= 0) {
>                  return 1;
> ===== st.c 1.45 vs edited =====
> --- 1.45/drivers/scsi/st.c	Fri Sep  5 12:16:40 2003
> +++ edited/drivers/scsi/st.c	Fri Nov 21 12:30:34 2003
> @@ -1267,7 +1267,8 @@
>  		i = STp->try_dio && try_rdio;
>  	else
>  		i = STp->try_dio && try_wdio;
> -	if (i) {
> +	if (i && ((unsigned int)buf & queue_dma_alignment(
> +					STp->device->request_queue)) == 0) {
>  		i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg,
>  				      (unsigned long)buf, count, (is_read ? READ : WRITE),
>  				      STp->max_pfn);
> ===== scsi_scan.c 1.52 vs edited =====
> --- 1.52/drivers/scsi/scsi_scan.c	Fri Oct 24 14:53:37 2003
> +++ edited/drivers/scsi/scsi_scan.c	Fri Nov 21 12:34:00 2003
> @@ -231,6 +231,15 @@
>  	sdev->request_queue->queuedata = sdev;
>  	scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
>  
> +	/*
> +	 * Set the queue's mask to require a mere 8-byte alignment for
> +	 * DMA buffers, rather than the default 512.  This shouldn't
> +	 * inconvenience any user programs and should be okay for most
> +	 * host adapters.  A host driver can alter this mask in its
> +	 * slave_alloc() or slave_configure() callback if necessary.
> +	 */
> +	blk_queue_dma_alignment(sdev->request_queue, (8 - 1));
> +
>  	if (shost->hostt->slave_alloc) {
>  		if (shost->hostt->slave_alloc(sdev))
>  			goto out_free_queue;

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

I want my GPFs!!!
					-- Stef
User Friendly, 11/9/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]