On Fri, Apr 06, 2018 at 02:59:16PM +0000, Bart Van Assche wrote: > On Fri, 2018-04-06 at 09:45 +0200, Johannes Thumshirn wrote: > > On 05/04/18 19:49, Martin K. Petersen wrote: > > > Longer term I'd really like to see the command result integer > > > host/driver/msg/status stuff cleaned up. It's super arcane and the > > > associated naming schemes make it a very error-prone interface. > > > > > > > I did start a series [1] for this but than got distracted by more urgent > > things. I can pick it up again I think. > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git/log/?h=iscsi_DID_REQUEUE > > Hello Johannes, > > Since modifying how SCSI results are communicated from SCSI LLDs to the SCSI > core requires to touch all SCSI LLDs, please consider to include the following > changes in your patch series: > - Remove the deprecated SCSI status codes (GOOD, CHECK_CONDITION, etc.) and > use the SAM_STAT_* symbolic names instead. I'll have a look, thanks for the heads up. > - Ensure that assigning an int to scsi_cmnd.result triggers a compiler error. > There is code in the Linux kernel that mixes traditional error codes (-EIO) > with the SCSI result codes (DID_ERROR << 16). I think most of that code is > buggy and that it should be fixed. Changing the type of scsi_cmnd.result is > one way to find such code. How about something like the following (untested) > patch? I'll give it a shot. > > diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h > index cb85eddb47ea..27f1c347f0ea 100644 > --- a/include/scsi/scsi.h > +++ b/include/scsi/scsi.h > @@ -18,6 +18,27 @@ enum scsi_timeouts { > SCSI_DEFAULT_EH_TIMEOUT = 10 * HZ, > }; > > +/* > + * Note: .result is signed because SCSI LLDs store a SCSI result in > + * .result while the bsg driver stores a negative error code in .result. > + */ > +typedef union { > + s32 result; > + struct { > +#if defined(__BIG_ENDIAN) > + u8 driver_byte; > + u8 host_byte; > + u8 msg_byte; > + u8 status_byte; > +#elif defined(__LITTLE_ENDIAN) > + u8 status_byte; > + u8 msg_byte; > + u8 host_byte; > + u8 driver_byte; > +#endif > + }; > +} scsi_result; > + > /* > * DIX-capable adapters effectively support infinite chaining for the > * protection information scatterlist > @@ -38,8 +59,10 @@ enum scsi_timeouts { > * This returns true for known good conditions that may be treated as > * command completed normally > */ > -static inline int scsi_status_is_good(int status) > +static inline int scsi_status_is_good(scsi_result result) > { > + u8 status = result.status_byte; > + > /* > * FIXME: bit0 is listed as reserved in SCSI-2, but is > * significant in SCSI-3. For now, we follow the SCSI-2 > @@ -205,10 +228,10 @@ static inline int scsi_is_wlun(u64 lun) > * host_byte = set by low-level driver to indicate status. > * driver_byte = set by mid-level. > */ > -#define status_byte(result) (((result) >> 1) & 0x7f) > -#define msg_byte(result) (((result) >> 8) & 0xff) > -#define host_byte(result) (((result) >> 16) & 0xff) > -#define driver_byte(result) (((result) >> 24) & 0xff) > +#define status_byte(result) ((result).status_byte >> 1) > +#define msg_byte(result) ((result).msg_byte) > +#define host_byte(result) ((result).host_byte) > +#define driver_byte(result) ((result).driver_byte) > > #define sense_class(sense) (((sense) >> 4) & 0x7) > #define sense_error(sense) ((sense) & 0xf) > diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h > index 2280b2351739..cbc5df948dd3 100644 > --- a/include/scsi/scsi_cmnd.h > +++ b/include/scsi/scsi_cmnd.h > @@ -144,7 +144,7 @@ struct scsi_cmnd { > * obtained by scsi_malloc is guaranteed > * to be at an address < 16Mb). */ > > - int result; /* Status code from lower level driver */ > + scsi_result result; /* Status code from lower level driver */ > int flags; /* Command flags */ > > unsigned char tag; /* SCSI-II queued command tag */ > > Bart. -- Johannes Thumshirn Storage jthumshirn@xxxxxxx +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
