On 23 August 2016 at 03:43, Shaun Tancheff <shaun.tancheff@xxxxxxxxxxx> wrote:
>>> + if (unmap) {
>>> + /* If trim is not enabled the cmd is invalid. */
>>> + if ((dev->horkage & ATA_HORKAGE_NOTRIM) ||
>>> + !ata_id_has_trim(dev->id)) {
>>> + fp = 1;
>>> + bp = 3;
>>> + goto invalid_fld;
>>> + }
>>> + /* If the request is too large the cmd is invalid */
>>> + if (n_block > 0xffff * trmax) {
>>> + fp = 2;
>>> + goto invalid_fld;
>>> + }
>>
>> This response should be generally applied to the Write Same (16)
>> translation, since it is required by SBC,
>>
>>> + } else {
>>> + /* If write same is not available the cmd is invalid */
>>> + if (!ata_id_sct_write_same(dev->id)) {
>>> + fp = 1;
>>> + bp = 3;
>>> + goto invalid_fld;
>>> + }
>>
>> therefore, you should add an n_block check here as well, if you are
>> going to advertise an Maximum Write Same Length even when the device
>> supports only SCT Write Same but not TRIM. Most likely you would want
>> to simply move the existing check one-level up (if the same limit is
>> advertised no matter TRIM is supported not or not).
>
> Why would we enforce upper level limits on something that doesn't
> have any?
If we advertise a limit in our SATL, it makes sense that we should
make sure the behaviour is consistent when we issue a write same
through the block layer / ioctl and when we issue a SCSI Write Same
command directly (e.g. with sg_write_same). IMHO that's pretty much
why SBC would mandate such behaviour as well.
>
> If the upper level, or SG_IO, chooses to set a timeout of 10 hours and
> wipe a whole disk it should be free to do so.
>
That's why I said, "if you are going to advertise an Maximum Write Same Length".
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
