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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
