Re: [PATCH 000/117] Make better use of static type checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/19/21 11:04 PM, Hannes Reinecke wrote:
> We should not try to preserve the split SCSI result value with its four
> distinct fields.

I don't think that we have the freedom to drop the four byte SCSI result
entirely since multiple user space APIs use that data structure. The
four-byte SCSI result value is embedded in the following user space API
data structures (there may be others):
* struct sg_io_v4, the SG_IO header includes the driver_status
(driver_byte()), transport_status (host_byte()) and device_status
(scsi_status & 0xff) (the message byte is not included).
* struct fc_bsg_reply.
* struct iscsi_bsg_reply.
* struct ufs_bsg_reply.

> I'd rather have the message byte handling moved into the SCSI parallel
> drivers (where really it should've been in the first place).
> The driver byte can go entirely as the DRIVER_SENSE flag can be replaced
> with a check for valid sense code, DRIVER_TIMEOUT is pretty much
> identical to DID_TIMEOUT (with the semantic difference _who_ set the
> timeout), and DRIVER_ERROR can be folded back into the caller.
> All other values are unused, allowing us to drop driver error completely.
> 
> With that we're only having two fields (host byte and status byte) left,
> which should be treated as two distinct values.
> 
> As it so happens I do have a patchset for this; guess I'll be posting it
> to demonstrate the idea.

This patch series does not prevent the conversion described above.
Although I think that the changes described above would help, I have a
few concerns:
- Some drivers use the result member of struct scsi_cmnd for another
purpose than storing SCSI result values. See e.g. the CAM_* values
defined in drivers/scsi/aic7xxx/cam.h and the use of these values in
drivers/scsi/aic7xxx/aic79xx_osm.c. From the master branch:

        [ ... ]
	cmd->scsi_done = scsi_done;
	cmd->result = CAM_REQ_INPROG << 16;
	rtn = ahd_linux_run_command(ahd, dev, cmd);
        [ ... ]

        [ ... ]
	if ((cmd->result & (CAM_DEV_QFRZN << 16)) != 0) {
		cmd->result &= ~(CAM_DEV_QFRZN << 16);
		dev->qfrozen--;
	}
        [ ... ]

Converting this driver could be challenging and may end up in rewriting
this driver.
- The SCSI drivers that do something meaningful with the message byte
are parallel SCSI drivers. Parallel SCSI is a technology that was
popular 20-30 years ago but that is no longer popular today. Finding
test hardware may be a big challenge.
- The parallel SCSI technology is no longer commercially relevant. It
may be challenging to motivate people (including yourself) to convert a
significant number of parallel SCSI drivers that each have a small user
base.

Although I appreciate it that you have shared this proposal and also
that you have proposed to lead this effort, I'm not convinced that this
proposal will be implemented soon. So I propose to proceed with this
patch series.

Thanks,

Bart.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux