Re: [PATCH v2] Add support for SCT Write Same

[Christoph Hellwig <hch@infradead.org>]

> +	if (ata_id_sct_write_same(dev->id))
> +		sdev->sct_write_same = 1;
> +

What's the point of this flag?  It should simply clear the no_write_same
flag for this device.  Due to the way how we have both a per-host and
per-device flag that might not be completely trivial, but untangling
that mess might be a good idea anyway.

> @@ -3305,6 +3308,37 @@ 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)));
> +
> +	if (ata_id_sct_write_same(dev->id)) {

Various comments:

 - The plain page_address above looks harmful, how do we know that
   the page is mapped into kernel memory?  This might actually be broken
   already, though.
 - Why is this below the check that rejects non-unmap WRITE SAME
   commands?
 - Shouldn't we still translate discard command to TRIM?  Maybe we
   need a check of the operation in the request structure..