| Message ID | 577b57b0.5b4c620a.37648.6c8d@mx.google.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 7/5/2016 9:45 AM, tom.ty89@gmail.com wrote: > From: Tom Yan <tom.ty89@gmail.com> > > Define TRIM_RANGE_SIZE and TRIM_RANGE_NUM so that the corresponding > functions can be more generalized. Also, conform to SBC by rejecting > WRITE SAME (16) commands with number of blocks that exceeds the limit > that is defined in the SATL. > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> [...] > diff --git a/include/linux/ata.h b/include/linux/ata.h > index 99346be..0971c3f 100644 > --- a/include/linux/ata.h > +++ b/include/linux/ata.h > @@ -1071,7 +1071,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, > __le64 *buffer = _buffer; > unsigned i = 0, used_bytes; > > - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ > + while (i < buf_size) { BTW, this change doesn't seem to be documented in the patch description? [...] MBR, Sergei -- 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
Well it is exactly what the patch does. We no longer do the "divide it
by 8" thing each time we want to know the now defined TRIM_RANGE_NUM.
ata_set_lba_range_entries() has only been used by
ata_scsi_write_same_xlat()
(http://lxr.free-electrons.com/ident?i=ata_set_lba_range_entries,
although the comment of it seems to tell otherwise). That's why I
(could) changed the function itself and the param ("512")
ata_scsi_write_same_xlat() passes to it.
On 5 July 2016 at 19:04, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 7/5/2016 9:45 AM, tom.ty89@gmail.com wrote:
>
>> From: Tom Yan <tom.ty89@gmail.com>
>>
>> Define TRIM_RANGE_SIZE and TRIM_RANGE_NUM so that the corresponding
>> functions can be more generalized. Also, conform to SBC by rejecting
>> WRITE SAME (16) commands with number of blocks that exceeds the limit
>> that is defined in the SATL.
>>
>> Signed-off-by: Tom Yan <tom.ty89@gmail.com>
>
>
> [...]
>
>> diff --git a/include/linux/ata.h b/include/linux/ata.h
>> index 99346be..0971c3f 100644
>> --- a/include/linux/ata.h
>> +++ b/include/linux/ata.h
>> @@ -1071,7 +1071,7 @@ static inline unsigned
>> ata_set_lba_range_entries(void *_buffer,
>> __le64 *buffer = _buffer;
>> unsigned i = 0, used_bytes;
>>
>> - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry
>> */
>> + while (i < buf_size) {
>
>
> BTW, this change doesn't seem to be documented in the patch description?
>
> [...]
>
> MBR, Sergei
>
--
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
On Wed, Jul 06, 2016 at 03:10:51AM +0800, Tom Yan wrote: > Well it is exactly what the patch does. We no longer do the "divide it > by 8" thing each time we want to know the now defined TRIM_RANGE_NUM. > > ata_set_lba_range_entries() has only been used by > ata_scsi_write_same_xlat() > (http://lxr.free-electrons.com/ident?i=ata_set_lba_range_entries, > although the comment of it seems to tell otherwise). That's why I > (could) changed the function itself and the param ("512") > ata_scsi_write_same_xlat() passes to it. Heh, I find the patch difficult to follow too and it shouldn't be. Can you please split them up and explain what's going on in the description? Thanks.
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..029e738 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -75,6 +75,9 @@ static struct ata_device *ata_scsi_find_dev(struct ata_port *ap, #define ALL_MPAGES 0x3f #define ALL_SUB_MPAGES 0xff +#define TRIM_RANGE_SIZE 0xffff +#define TRIM_RANGE_NUM 64 /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ + static const u8 def_rw_recovery_mpage[RW_RECOVERY_MPAGE_LEN] = { RW_RECOVERY_MPAGE, @@ -2314,7 +2317,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(TRIM_RANGE_SIZE * TRIM_RANGE_NUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3305,7 +3308,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= TRIM_RANGE_SIZE * TRIM_RANGE_NUM) + size = ata_set_lba_range_entries(buf, TRIM_RANGE_NUM, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..0971c3f 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -1071,7 +1071,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0xffff ? 0xffff : count) << 48); buffer[i++] = __cpu_to_le64(entry);