> -----Original Message----- > From: Christoph Hellwig [mailto:hch@xxxxxx] > Sent: Thursday, 30 October, 2014 4:27 AM > To: linux-scsi@xxxxxxxxxxxxxxx > Cc: Douglas Gilbert; Elliott, Robert (Server Storage) > Subject: [PATCH 2/6] scsi: split scsi_nonblockable_ioctl > > The calling conventions for this function where bad as it could > return > -ENODEV both for a device not currently online and a not recognized > ioctl. > > Add a new scsi_ioctl_block_when_processing_errors function that wraps > scsi_block_when_processing_errors with the a special case for the > SG_SCSI_RESET ioctl command, and handle the SG_SCSI_RESET case itself > in scsi_ioctl. All callers of scsi_ioctl now must call the above > helper to check for the EH state, so that the ioctl handler itself doesn't > have to. > > Reported-by: Robert Elliott <Elliott@xxxxxx> > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > drivers/scsi/ch.c | 5 +++++ > drivers/scsi/osst.c | 6 +++--- > drivers/scsi/scsi_ioctl.c | 47 +++++++++++++------------------------ > ---------- > drivers/scsi/sd.c | 6 +++--- > drivers/scsi/sg.c | 33 +++++++++++++++------------------ > drivers/scsi/sr.c | 15 +++++---------- > drivers/scsi/st.c | 7 +++---- > include/scsi/scsi_ioctl.h | 4 ++-- > 8 files changed, 49 insertions(+), 74 deletions(-) > > diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c > index 5ddc08f..712f159 100644 > --- a/drivers/scsi/scsi_ioctl.c > +++ b/drivers/scsi/scsi_ioctl.c ... > @@ -281,30 +270,20 @@ int scsi_ioctl(struct scsi_device *sdev, int > cmd, void __user *arg) > } > EXPORT_SYMBOL(scsi_ioctl); > > -/** > - * scsi_nonblockable_ioctl() - Handle SG_SCSI_RESET > - * @sdev: scsi device receiving ioctl > - * @cmd: Must be SC_SCSI_RESET > - * @arg: pointer to int containing > SG_SCSI_RESET_{DEVICE,TARGET,BUS,HOST} > - * possibly OR-ed with SG_SCSI_RESET_NO_ESCALATE > - * @ndelay: file mode O_NDELAY flag > +/* > + * We can process a reset even when a device isn't fully operable. > */ > -int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd, > - void __user *arg, int ndelay) > +int scsi_ioctl_block_when_processing_errors(struct scsi_device > *sdev, int cmd, > + bool ndelay) > { > - /* The first set of iocts may be executed even if we're doing > - * error processing, as long as the device was opened > - * non-blocking */ > - if (ndelay) { > + if (cmd == SG_SCSI_RESET && ndelay) { > if (scsi_host_in_recovery(sdev->host)) > return -ENODEV; > - } else if (!scsi_block_when_processing_errors(sdev)) > - return -ENODEV; > - > - switch (cmd) { > - case SG_SCSI_RESET: > - return scsi_ioctl_reset(sdev, arg); > + } else { > + if (!scsi_block_when_processing_errors(sdev)) > + return -ENODEV; > } > - return -ENODEV; > + > + return 0; > } > -EXPORT_SYMBOL(scsi_nonblockable_ioctl); > +EXPORT_SYMBOL_GPL(scsi_ioctl_block_when_processing_errors); Most of the SCSI midlayer functions are exported as non-GPL; is it really necessary to prevent closed source drivers from using this new function, especially since it's just a refactoring of some previously non-GPL exported functions? ... > diff --git a/include/scsi/scsi_ioctl.h b/include/scsi/scsi_ioctl.h > index b900684..8d19d1d 100644 > --- a/include/scsi/scsi_ioctl.h > +++ b/include/scsi/scsi_ioctl.h > @@ -40,9 +40,9 @@ typedef struct scsi_fctargaddress { > unsigned char host_wwn[8]; // include NULL term. > } Scsi_FCTargAddress; > > +int scsi_ioctl_block_when_processing_errors(struct scsi_device > *sdev, > + int cmd, bool ndelay); > extern int scsi_ioctl(struct scsi_device *, int, void __user *); > -extern int scsi_nonblockable_ioctl(struct scsi_device *sdev, int > cmd, > - void __user *arg, int ndelay); > > #endif /* __KERNEL__ */ > #endif /* _SCSI_IOCTL_H */ > -- > 1.9.1 Beyond this patch, the existing scsi_block_when_processing_errors description should be expanded to mention that it is used to prevent ioctls and commands, not just commands: scsi_error.c: /** * scsi_block_when_processing_errors - Prevent cmds from being queued. * @sdev: Device on which we are performing recovery. * * Description: * We block until the host is out of error recovery, and then check to * see whether the host or the device is offline. * * Return value: * 0 when dev was taken offline by error recovery. 1 OK to proceed. */ int scsi_block_when_processing_errors(struct scsi_device *sdev) ... Reviewed-by: Robert Elliott <elliott@xxxxxx> --- Rob Elliott HP Server Storage -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
