On 10/8/19 9:50 AM, Douglas Gilbert wrote:
> The introduced request state machine is not wired in so that
> the size of one of the following patches is reduced. Bit
> operation defines for the request and file descriptor level
> are also introduced. Minor rework og sg_rd_append() function.
>
> Signed-off-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
> ---
> drivers/scsi/sg.c | 237 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 175 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 5b560f720993..4e6f6fb2a54e 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -75,7 +75,43 @@ static char *sg_version_date = "20190606";
> #define uptr64(val) ((void __user *)(uintptr_t)(val))
> #define cuptr64(val) ((const void __user *)(uintptr_t)(val))
>
> +/* Following enum contains the states of sg_request::rq_st */
> +enum sg_rq_state {
> + SG_RS_INACTIVE = 0, /* request not in use (e.g. on fl) */
> + SG_RS_INFLIGHT, /* active: cmd/req issued, no response yet */
> + SG_RS_AWAIT_RD, /* response received, awaiting read */
> + SG_RS_DONE_RD, /* read is ongoing or done */
> + SG_RS_BUSY, /* temporary state should rarely be seen */
> +};
> +
> +#define SG_TIME_UNIT_MS 0 /* milliseconds */
> +#define SG_DEF_TIME_UNIT SG_TIME_UNIT_MS
> #define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
> +#define SG_FD_Q_AT_HEAD 0
> +#define SG_DEFAULT_Q_AT SG_FD_Q_AT_HEAD /* for backward compatibility */
> +#define SG_FL_MMAP_DIRECT (SG_FLAG_MMAP_IO | SG_FLAG_DIRECT_IO)
> +
> +/* Only take lower 4 bits of driver byte, all host byte and sense byte */
> +#define SG_ML_RESULT_MSK 0x0fff00ff /* mid-level's 32 bit result value */
> +
> +#define SG_PACK_ID_WILDCARD (-1)
> +
> +#define SG_ADD_RQ_MAX_RETRIES 40 /* to stop infinite _trylock(s) */
> +
> +/* Bit positions (flags) for sg_request::frq_bm bitmask follow */
> +#define SG_FRQ_IS_ORPHAN 1 /* owner of request gone */
> +#define SG_FRQ_SYNC_INVOC 2 /* synchronous (blocking) invocation */
> +#define SG_FRQ_DIO_IN_USE 3 /* false->indirect_IO,mmap; 1->dio */
> +#define SG_FRQ_NO_US_XFER 4 /* no user space transfer of data */
> +#define SG_FRQ_DEACT_ORPHAN 7 /* not keeping orphan so de-activate */
> +#define SG_FRQ_BLK_PUT_REQ 9 /* set when blk_put_request() called */
> +
> +/* Bit positions (flags) for sg_fd::ffd_bm bitmask follow */
> +#define SG_FFD_FORCE_PACKID 0 /* receive only given pack_id/tag */
> +#define SG_FFD_CMD_Q 1 /* clear: only 1 active req per fd */
> +#define SG_FFD_KEEP_ORPHAN 2 /* policy for this fd */
> +#define SG_FFD_MMAP_CALLED 3 /* mmap(2) system call made on fd */
> +#define SG_FFD_Q_AT_TAIL 5 /* set: queue reqs at tail of blk q */
>
> /* Bit positions (flags) for sg_device::fdev_bm bitmask follow */
> #define SG_FDEV_EXCLUDE 0 /* have fd open with O_EXCL */
> @@ -83,12 +119,11 @@ static char *sg_version_date = "20190606";
> #define SG_FDEV_LOG_SENSE 2 /* set by ioctl(SG_SET_DEBUG) */
>
> int sg_big_buff = SG_DEF_RESERVED_SIZE;
> -/* N.B. This variable is readable and writeable via
> - /proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
> - of this size (or less if there is not enough memory) will be reserved
> - for use by this file descriptor. [Deprecated usage: this variable is also
> - readable via /proc/sys/kernel/sg-big-buff if the sg driver is built into
> - the kernel (i.e. it is not a module).] */
> +/*
> + * This variable is accessible via /proc/scsi/sg/def_reserved_size . Each
> + * time sg_open() is called a sg_request of this size (or less if there is
> + * not enough memory) will be reserved for use by this file descriptor.
> + */
> static int def_reserved_size = -1; /* picks up init parameter */
> static int sg_allow_dio = SG_ALLOW_DIO_DEF;
>
> @@ -132,6 +167,7 @@ struct sg_request { /* SG_MAX_QUEUE requests outstanding per file */
> char sg_io_owned; /* 1 -> packet belongs to SG_IO */
> /* done protected by rq_list_lock */
> char done; /* 0->before bh, 1->before read, 2->read */
> + atomic_t rq_st; /* request state, holds a enum sg_rq_state */
> struct request *rq; /* released in sg_rq_end_io(), bio kept */
> struct bio *bio; /* kept until this req -->SG_RS_INACTIVE */
> struct execute_work ew_orph; /* harvest orphan request */
> @@ -208,10 +244,15 @@ static void sg_unlink_reserve(struct sg_fd *sfp, struct sg_request *srp);
> static struct sg_fd *sg_add_sfp(struct sg_device *sdp);
> static void sg_remove_sfp(struct kref *);
> static struct sg_request *sg_add_request(struct sg_fd *sfp);
> -static int sg_deact_request(struct sg_fd *sfp, struct sg_request *srp);
> +static void sg_deact_request(struct sg_fd *sfp, struct sg_request *srp);
> static struct sg_device *sg_get_dev(int dev);
> static void sg_device_destroy(struct kref *kref);
> static void sg_calc_sgat_param(struct sg_device *sdp);
> +static const char *sg_rq_st_str(enum sg_rq_state rq_st, bool long_str);
> +static void sg_rep_rq_state_fail(struct sg_fd *sfp,
> + enum sg_rq_state exp_old_st,
> + enum sg_rq_state want_st,
> + enum sg_rq_state act_old_st);
>
> #define SZ_SG_HEADER ((int)sizeof(struct sg_header)) /* v1 and v2 header */
> #define SZ_SG_IO_HDR ((int)sizeof(struct sg_io_hdr)) /* v3 header */
> @@ -219,6 +260,8 @@ static void sg_calc_sgat_param(struct sg_device *sdp);
>
> #define SG_IS_DETACHING(sdp) test_bit(SG_FDEV_DETACHING, (sdp)->fdev_bm)
> #define SG_HAVE_EXCLUDE(sdp) test_bit(SG_FDEV_EXCLUDE, (sdp)->fdev_bm)
> +#define SG_RS_ACTIVE(srp) (atomic_read(&(srp)->rq_st) != SG_RS_INACTIVE)
> +#define SG_RS_AWAIT_READ(srp) (atomic_read(&(srp)->rq_st) == SG_RS_AWAIT_RD)
>
> /*
> * Kernel needs to be built with CONFIG_SCSI_LOGGING to see log messages.
> @@ -382,15 +425,6 @@ sg_open(struct inode *inode, struct file *filp)
> res = sg_allow_if_err_recovery(sdp, non_block);
> if (res)
> goto error_out;
> - /* scsi_block_when_processing_errors() may block so bypass
> - * check if O_NONBLOCK. Permits SCSI commands to be issued
> - * during error recovery. Tread carefully. */
> - if (!((op_flags & O_NONBLOCK) ||
> - scsi_block_when_processing_errors(sdp->device))) {
> - res = -ENXIO;
> - /* we are in error recovery for this device */
> - goto error_out;
> - }
>
> mutex_lock(&sdp->open_rel_lock);
> if (op_flags & O_NONBLOCK) {
> @@ -494,12 +528,12 @@ sg_write(struct file *filp, const char __user *p, size_t count, loff_t *ppos)
> struct sg_device *sdp;
> struct sg_fd *sfp;
> struct sg_request *srp;
> + u8 cmnd[SG_MAX_CDB_SIZE];
> struct sg_header ov2hdr;
> struct sg_io_hdr v3hdr;
> struct sg_header *ohp = &ov2hdr;
> struct sg_io_hdr *h3p = &v3hdr;
> struct sg_comm_wr_t cwr;
> - u8 cmnd[SG_MAX_CDB_SIZE];
>
> res = sg_check_file_access(filp, __func__);
> if (res)
> @@ -748,11 +782,25 @@ sg_common_write(struct sg_fd *sfp, struct sg_comm_wr_t *cwrp)
> return 0;
> }
>
> +static inline int
> +sg_rstate_chg(struct sg_request *srp, enum sg_rq_state old_st,
> + enum sg_rq_state new_st)
> +{
> + enum sg_rq_state act_old_st = (enum sg_rq_state)
> + atomic_cmpxchg(&srp->rq_st, old_st, new_st);
> +
> + if (act_old_st == old_st)
> + return 0; /* implies new_st --> srp->rq_st */
> + else if (IS_ENABLED(CONFIG_SCSI_LOGGING))
> + sg_rep_rq_state_fail(srp->parentfp, old_st, new_st,
> + act_old_st);
> + return -EPROTOTYPE;
> +}
>
-EPROTOTYPE?
Now there someone has read POSIX ... why not simply -EINVAL as one would
expect?
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer
