| Message ID | 1258771524-26673-5-git-send-email-martin.petersen@oracle.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Nov 20, 2009 at 09:45:24PM -0500, Martin K. Petersen wrote: > ata_set_lba_range_entries confused indexes into a u64 buffer with byte > offsets. Fix this up and document the return value. > > Update the libata write same translation to reflect this. Once we fix up this beast we should also change the current max argument to encode the buffer size, not the maximum number of entries which is a rather confusing interface. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes: Christoph> On Fri, Nov 20, 2009 at 09:45:24PM -0500, Martin K. Petersen wrote: >> ata_set_lba_range_entries confused indexes into a u64 buffer with >> byte offsets. Fix this up and document the return value. >> >> Update the libata write same translation to reflect this. Christoph> Once we fix up this beast we should also change the current Christoph> max argument to encode the buffer size, not the maximum Christoph> number of entries which is a rather confusing interface. What threw me off yesterday was that `max' starts out being a cap on the number of 8-byte entries in the buffer. And then towards the end it suddenly turns into a number-of-bytes entity as a result of the ALIGN macro. I agree that we should fix this up to be consistent. I'll do that.
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 08d4ab7..0c496f8 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2950,7 +2950,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) const u8 *cdb = scmd->cmnd; u64 block; u32 n_block; - u32 size; + u32 trim_sectors; void *buf; /* we may not issue DMA commands if no DMA mode is set */ @@ -2973,13 +2973,13 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_fld; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512 / 8, block, n_block); + trim_sectors = ata_set_lba_range_entries(buf, 512 / 8, block, n_block); tf->protocol = ATA_PROT_DMA; tf->hob_feature = 0; tf->feature = ATA_DSM_TRIM; - tf->hob_nsect = (size / 512) >> 8; - tf->nsect = size / 512; + tf->hob_nsect = trim_sectors >> 8; + tf->nsect = trim_sectors; tf->command = ATA_CMD_DSM; tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_WRITE; diff --git a/include/linux/ata.h b/include/linux/ata.h index b18b2bb..150c9dc 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -983,7 +983,8 @@ static inline void ata_id_to_hd_driveid(u16 *id) /* * Write up to 'max' LBA Range Entries to the buffer that will cover the * extent from sector to sector + count. This is used for TRIM and for - * ADD LBA(S) TO NV CACHE PINNED SET. + * ADD LBA(S) TO NV CACHE PINNED SET. Returns number of 512-byte + * sectors required to encode the range. */ static inline unsigned ata_set_lba_range_entries(void *_buffer, unsigned max, u64 sector, unsigned long count) @@ -1001,9 +1002,9 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, unsigned max, sector += 0xffff; } - max = ALIGN(i * 8, 512); - memset(buffer + i, 0, max - i * 8); - return max; + memset(&buffer[i], 0, (max - i) * 8); + + return ALIGN(i * 8, 512); } static inline int is_multi_taskfile(struct ata_taskfile *tf)
ata_set_lba_range_entries confused indexes into a u64 buffer with byte offsets. Fix this up and document the return value. Update the libata write same translation to reflect this. Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> --- drivers/ata/libata-scsi.c | 8 ++++---- include/linux/ata.h | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-)