Jens Axboe wrote:
> Hi Doug,
> 
> resp_inquiry() does a GFP_KERNEL memory allocation, but it's not allowed
> to from the queuecommand context. There's no good way to return this
> error, so I used DID_ERROR which is used from similar paths. That
> doesn't seem quite right though, it would be better to return an error
> indicating a later retry would be more appropriate.
> 
> Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 30ee3d7..0c80ed3 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -954,7 +954,9 @@ static int resp_inquiry(struct scsi_cmnd * scp, int target,
>  	int alloc_len, n, ret;
>  
>  	alloc_len = (cmd[3] << 8) + cmd[4];
> -	arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_KERNEL);
> +	arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
> +	if (!arr)
> +		return DID_ERROR << 16;
>  	if (devip->wlun)
>  		pq_pdt = 0x1e;	/* present, wlun */
>  	else if (scsi_debug_no_lun_0 && (0 == devip->lun))
> 

Jens,
I had to read that twice. I'm always happy to convert a
GFP_KERNEL to a GFP_ATOMIC (as I'm sure it started as a
GFP_ATOMIC). There are a couple more that may be in
queuecommand context.

Taking up your point about retries and seeing that the
scsi_debug driver has a SAS flavour, I'm inclined towards
a "aborted command, initiator response timeout" [Bh,4Bh,6]
CHECK CONDITION. There is a group of transport injected
error messages in SAS (see sas2r07.pdf section 10.2.3)
that pop up from time to time. Due to conjestion in
connection-switched SAS expanders these error messages
should be interpreted as "try again" depending on the
topology. The patch below adds a "aborted_command" bit
in opts that will cause every nth command to be aborted
(with an ack/nak timeout).

Note that SAS has an optional "transport layer retries"
state machine to lessen the incidence of "aborted commands".
Evidently SAS tape drives use the facility.

Doug Gilbert