Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

On Tue, 2008-01-22 at 15:32 -0500, Jeff Garzik wrote:

James Bottomley wrote:

quoted

Actually, I don't think it's a smaller I/O issue.  The SMART protocol
specifically mandates that the transfers for SMART READ DATA and SMART
READ LOG shall be 512 bytes).  However, the pio transfer routine does
seem to be assuming sector alignment as well, which will be where your
problems are coming from.  I think we need to specify sector minimum
alignment for ata (but not atapi, which has its own non sector size pio
routine).  How about the attached?

We have to do this for all ATA devices, because they'll likely all
support SMART, and SMART is defined to be a PIO command.

James

---

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4bb268b..bc5cf6b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c

@@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
 	 * requests.
 	 */
 	sdev->max_device_blocked = 1;
-
-	/* set the min alignment */
-	blk_queue_update_dma_alignment(sdev->request_queue, ATA_DMA_PAD_SZ - 1);
 }
 
 static void ata_scsi_dev_config(struct scsi_device *sdev,

@@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
 	if (dev->class == ATA_DEV_ATAPI) {
 		struct request_queue *q = sdev->request_queue;
 		blk_queue_max_hw_segments(q, q->max_hw_segments - 1);
-	}
+
+		/* set the min alignment */
+		blk_queue_update_dma_alignment(sdev->request_queue,
+					       ATA_DMA_PAD_SZ - 1);
+	} else
+		/* ATA devices must be sector aligned */
+		blk_queue_update_dma_alignment(sdev->request_queue,
+					       ATA_SECT_SIZE - 1);
 
 	if (dev->flags & ATA_DFLAG_AN)
 		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);

ACK

Unlike ATAPI, ATA is indeed all 512-byte alignment transfers (_not_ 
sector size, which may or may not be 512 bytes)

Does this apply to libata?  libata + jejb dma alignment patch?

What tree...

It's scsi-misc-2.6; the blk_queue_update_dma_alignment() API doesn't
exist in mainline (yet).

James