Re: [PATCH v3] scsi: core: clear driver private data when retry request

[Bart Van Assche <bvanassche@xxxxxxx>]

On 2/18/25 4:13 AM, John Garry wrote:
> TBH, I am not sure on the history here. Maybe Bart or Christoph knows, 
> but my impression is still that the priv data is only cleared once in 
> the lifetime of the request (from 1bad6c4a) - at prep time - and some 
> drivers may rely on that (not be cleared again). Unlikely, though.

I'm not aware of any such drivers.

Driver-private data was introduced together with the scsi-mq code. I'm
not aware of a similar concept in the legacy SCSI core.

Commit d285203cf647 ("scsi: add support for a blk-mq based I/O path")
introduced the following code in kernel v3.17-rc1:

+static int scsi_mq_prep_fn(struct request *req)
+{
[ ... ]
+       memset(cmd, 0, sizeof(struct scsi_cmnd));
[ ... ]
+static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
+{
[ ... ]
+       if (!(req->cmd_flags & REQ_DONTPREP)) {
+               ret = prep_to_mq(scsi_mq_prep_fn(req));
+               if (ret)
+                       goto out_dec_host_busy;
+               req->cmd_flags |= REQ_DONTPREP;
+       }

I think the above memset() call was introduced because of the following
code in the legacy SCSI core (from kernel v3.16):

struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t 
gfp_mask)
{
	struct scsi_cmnd *cmd = scsi_host_alloc_command(shost, gfp_mask);

	if (unlikely(!cmd)) {
		unsigned long flags;

		spin_lock_irqsave(&shost->free_list_lock, flags);
		if (likely(!list_empty(&shost->free_list))) {
			cmd = list_entry(shost->free_list.next,
					 struct scsi_cmnd, list);
			list_del_init(&cmd->list);
		}
		spin_unlock_irqrestore(&shost->free_list_lock, flags);

		if (cmd) {
			void *buf, *prot;

			buf = cmd->sense_buffer;
			prot = cmd->prot_sdb;

			memset(cmd, 0, sizeof(*cmd));

			cmd->sense_buffer = buf;
			cmd->prot_sdb = prot;
		}
	}

	return cmd;
}
EXPORT_SYMBOL_GPL(__scsi_get_command);

If I'm reading the v3.16 block layer and SCSI code correctly,
__scsi_get_command() was called not only when a command was submitted
but also when it got resubmitted. See also the q->prep_rq_fn() call in
blk_peek_request().

Since the historic behavior involved clearing the entire struct
scsi_cmnd during requeuing, I'm fine with restoring this behavior.

Thanks,

Bart.