Re: [PATCH] scsi: clear UAC before sending SG_IO

[Douglas Gilbert]

On 2020-09-10 6:15 a.m., Randall Huang wrote:
> Make sure UAC is clear before sending SG_IO.
>
> Signed-off-by: Randall Huang <huangrandall@xxxxxxxxxx>

This patch just looks wrong. Imagine if every LLD front loaded some LLD
specific code before each invocation of ioctl(SG_IO). Is UAC Unit Attention
Condition? If so the mid-level notes them as they fly past.

Haven't looked at the rest of the patchset but I suspect the "wlun_clr_uac"
work needs a rethink. If that is the REPORT LUNS Well known LUN then perhaps
it could be handled in the mid-level scanning code. Otherwise it should
be handled in the LLD/UFS.

Also users of ioctl(SG_IO) should be capable of handling UAs, even if they
are irrelevant, and repeat the invocation. Finally ioctl(sg_dev, SG_IO) is
not the only way to send a pass-through command, there are also
  - write(sg_dev, ...)
  - ioctl(bsg_dev, SG_IO, ...)
  - ioctl(most_blk_devs, SG_IO, ...)
  - ioctl(st_dev, SG_IO, ...)

Hopefully I have convinced you by now not to take this route.

Doug Gilbert

> ---
>   drivers/scsi/sg.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 20472aaaf630..ad11bca47ae8 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -922,6 +922,7 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>   	int result, val, read_only;
>   	Sg_request *srp;
>   	unsigned long iflags;
> +	int _cmd;
>   
>   	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
>   				   "sg_ioctl: cmd=0x%x\n", (int) cmd_in));
> @@ -933,6 +934,13 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>   			return -ENODEV;
>   		if (!scsi_block_when_processing_errors(sdp->device))
>   			return -ENXIO;
> +
> +		_cmd = SCSI_UFS_REQUEST_SENSE;
> +		if (sdp->device->host->wlun_clr_uac) {
> +			sdp->device->host->hostt->ioctl(sdp->device, _cmd, NULL);
> +			sdp->device->host->wlun_clr_uac = false;
> +		}
> +
>   		result = sg_new_write(sfp, filp, p, SZ_SG_IO_HDR,
>   				 1, read_only, 1, &srp);
>   		if (result < 0)