| Message ID | 5659242e8546753768944757fc86988f1986585c.1468384890.git.tom.ty89@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Wed, Jul 13, 2016 at 12:47:08PM +0800, tom.ty89@gmail.com wrote: > From: Tom Yan <tom.ty89@gmail.com> > > As of commit 6b7e9cde4969 ("sd: Fix rw_max for devices that report > an optimal xfer size"), the scsi disk driver (correctly) derive both > of the queue limits "io_opt" and "max_sectors" from the optimal > transfer length field. > > In case we would like the two limits to be derived from a value > other than BLK_DEF_MAX_SECTORS for ATA disks in the future, this > patch has made it easy. What's the actual impact of this patch at this point? Thanks.
Well it's mainly to "pre-de-couple" BLK_DEF_MAX_SECTORS from ATA drives. If we left the field unset, the scsi disk driver will fall back to BLK_DEF_MAX_SECTORS when setting the block limit "max_sectors" (while its cap "max_hw_sectors" is set by libata depending on whether the drive supports LBA48 or so). So by filling in this field, we can set an optimal value for the ATA drives specifically, while the block layer devs will have less to concern when they determine the value of the macro (which should be hardware-neutural, sort of). A "side-effect" of this is, the scsi disk driver will also use this field to set the block limit "io_opt", which we had left it unset all the time. I am not sure about if there are any pros and cons in setting the limit, that's why I sent this as an RFC and to the linux-block mailing list as well. By looking at what the scsi disk driver do, it appears that it's a sensible thing to set the limit to a value that matches with the other limit "max_sectors", though. On 14 July 2016 at 01:04, Tejun Heo <tj@kernel.org> wrote: > On Wed, Jul 13, 2016 at 12:47:08PM +0800, tom.ty89@gmail.com wrote: >> From: Tom Yan <tom.ty89@gmail.com> >> >> As of commit 6b7e9cde4969 ("sd: Fix rw_max for devices that report >> an optimal xfer size"), the scsi disk driver (correctly) derive both >> of the queue limits "io_opt" and "max_sectors" from the optimal >> transfer length field. >> >> In case we would like the two limits to be derived from a value >> other than BLK_DEF_MAX_SECTORS for ATA disks in the future, this >> patch has made it easy. > > What's the actual impact of this patch at this point? > > Thanks. > > -- > tejun -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..ab75b5e 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2305,6 +2305,13 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) put_unaligned_be16(min_io_sectors, &rbuf[6]); /* + * Optimal transfer length. + * + * This is used to derive the queue limit "max_sector" and "io_opt". + */ + put_unaligned_be32(BLK_DEF_MAX_SECTORS, &rbuf[12]); + + /* * Optimal unmap granularity. * * The ATA spec doesn't even know about a granularity or alignment