Re: [PATCH 06/10] sd: don't use scsi_setup_blk_pc_cmnd for discard requests

[Christoph Hellwig <hch@lst.de>]

On Mon, Jul 07, 2014 at 12:01:53AM +0000, Elliott, Robert (Server Storage) wrote:
> >  	nr_sectors >>= ilog2(sdp->sector_size) - 9;
> > -	rq->timeout = SD_TIMEOUT;
> > -
> > -	memset(rq->cmd, 0, rq->cmd_len);
> 
> Is *cmd guaranteed to be zero at this point?

Yes, scsi_setup_fs_cmnd takes care of this before calling into the driver.

> With scsi-mq.2 I've seen UNMAP CDBs with garbage in CDB bytes 
> 2 to 5 (which causes a drive that checks reserved fields to
> reject the command), which suggests there is a case in which 
> that is not guaranteed.

Indeed, we only do it up to the actual CDB length, which is due to:

	sd: don't use rq->cmd_len before setting it up

I had to add this because blk-mq doesn't initialize cmd_len, unlike the
old path.

Either we need to merge this series as well to take care of all this,
or I should do a replacement for the above patch that just memsets up to
BLK_MAX_CDB.

> If so, then getting rid of the double memset is another 
> benefit, and removing it from scsi_setup_fs_cmnd would
> be good too.  Since that function is EXPORTED, does that
> prevent removing its memset?

We do need one of the memsets.  With this series I'm keeping the one
in scsi_setup_fs_cmnd so that it's done in one single place, which
seems preferable over having it duplicated in various spots.  At the
end of the series scsi_setup_fs_cmnd ceases to be exported.