Message ID | 20200703130122.111448-3-hare@suse.de (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | scsi: enable reserved commands for LLDDs | expand |
On 03/07/2020 14:01, Hannes Reinecke wrote: +linux-block I figure that linux-block should be cc'ed here > Some drivers require to allocate requests for internal command > submission. These request will never be passed through the block > layer, but nevertheless require a valid tag to avoid them clashing > with normal I/O commands. > This patch adds a new request flag REQ_INTERNAL to mark such > requests and a terminates any such commands in blk_execute_rq_nowait() > with a WARN_ON_ONCE to signal such an invalid usage. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > block/blk-exec.c | 5 +++++ > include/linux/blk_types.h | 2 ++ > include/linux/blkdev.h | 5 +++++ > 3 files changed, 12 insertions(+) > > diff --git a/block/blk-exec.c b/block/blk-exec.c > index 85324d53d072..6869877e0d21 100644 > --- a/block/blk-exec.c > +++ b/block/blk-exec.c > @@ -55,6 +55,11 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, > rq->rq_disk = bd_disk; > rq->end_io = done; > > + if (WARN_ON_ONCE(blk_rq_is_internal(rq))) { > + blk_mq_end_request(rq, BLK_STS_NOTSUPP); > + return; > + } > + > blk_account_io_start(rq); > > /* > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index ccb895f911b1..e386c43e4d77 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -360,6 +360,7 @@ enum req_flag_bits { > /* for driver use */ > __REQ_DRV, > __REQ_SWAP, /* swapping request. */ > + __REQ_INTERNAL, /* driver-internal command */ > __REQ_NR_BITS, /* stops here */ > }; > > @@ -384,6 +385,7 @@ enum req_flag_bits { > > #define REQ_DRV (1ULL << __REQ_DRV) > #define REQ_SWAP (1ULL << __REQ_SWAP) > +#define REQ_INTERNAL (1ULL << __REQ_INTERNAL) > > #define REQ_FAILFAST_MASK \ > (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER) > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 8fd900998b4e..d09210d4591e 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -273,6 +273,11 @@ static inline bool blk_rq_is_passthrough(struct request *rq) > return blk_rq_is_scsi(rq) || blk_rq_is_private(rq); > } > > +static inline bool blk_rq_is_internal(struct request *rq) > +{ > + return rq->cmd_flags & REQ_INTERNAL; > +} > + > static inline bool bio_is_passthrough(struct bio *bio) > { > unsigned op = bio_op(bio); >
On 7/8/20 3:27 AM, John Garry wrote: > On 03/07/2020 14:01, Hannes Reinecke wrote: > > +linux-block > > I figure that linux-block should be cc'ed here > >> Some drivers require to allocate requests for internal command >> submission. These request will never be passed through the block >> layer, but nevertheless require a valid tag to avoid them clashing >> with normal I/O commands. >> This patch adds a new request flag REQ_INTERNAL to mark such >> requests and a terminates any such commands in blk_execute_rq_nowait() >> with a WARN_ON_ONCE to signal such an invalid usage. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> block/blk-exec.c | 5 +++++ >> include/linux/blk_types.h | 2 ++ >> include/linux/blkdev.h | 5 +++++ >> 3 files changed, 12 insertions(+) >> >> diff --git a/block/blk-exec.c b/block/blk-exec.c >> index 85324d53d072..6869877e0d21 100644 >> --- a/block/blk-exec.c >> +++ b/block/blk-exec.c >> @@ -55,6 +55,11 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, >> rq->rq_disk = bd_disk; >> rq->end_io = done; >> >> + if (WARN_ON_ONCE(blk_rq_is_internal(rq))) { >> + blk_mq_end_request(rq, BLK_STS_NOTSUPP); >> + return; >> + } >> + >> blk_account_io_start(rq); The whole concept seems very odd, and then there's this seemingly randomly placed check and error condition. As I haven't seen the actual use case for this, hard to make suggestions though.
On 09/07/2020 21:02, Jens Axboe wrote: > On 7/8/20 3:27 AM, John Garry wrote: >> On 03/07/2020 14:01, Hannes Reinecke wrote: >> >> +linux-block >> >> I figure that linux-block should be cc'ed here >> >>> Some drivers require to allocate requests for internal command >>> submission. These request will never be passed through the block >>> layer, but nevertheless require a valid tag to avoid them clashing >>> with normal I/O commands. >>> This patch adds a new request flag REQ_INTERNAL to mark such >>> requests and a terminates any such commands in blk_execute_rq_nowait() >>> with a WARN_ON_ONCE to signal such an invalid usage. So if these requests will never be passed through the block layer - as you say - then why add a check for this? Surely you will find out in nasty and obvious ways that it should be done. Thanks, John >>> >>> Signed-off-by: Hannes Reinecke <hare@suse.de> >>> --- >>> block/blk-exec.c | 5 +++++ >>> include/linux/blk_types.h | 2 ++ >>> include/linux/blkdev.h | 5 +++++ >>> 3 files changed, 12 insertions(+) >>> >>> diff --git a/block/blk-exec.c b/block/blk-exec.c >>> index 85324d53d072..6869877e0d21 100644 >>> --- a/block/blk-exec.c >>> +++ b/block/blk-exec.c >>> @@ -55,6 +55,11 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, >>> rq->rq_disk = bd_disk; >>> rq->end_io = done; >>> >>> + if (WARN_ON_ONCE(blk_rq_is_internal(rq))) { >>> + blk_mq_end_request(rq, BLK_STS_NOTSUPP); >>> + return; >>> + } >>> + >>> blk_account_io_start(rq); > > The whole concept seems very odd, and then there's this seemingly > randomly placed check and error condition. As I haven't seen the > actual use case for this, hard to make suggestions though. >
diff --git a/block/blk-exec.c b/block/blk-exec.c index 85324d53d072..6869877e0d21 100644 --- a/block/blk-exec.c +++ b/block/blk-exec.c @@ -55,6 +55,11 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, rq->rq_disk = bd_disk; rq->end_io = done; + if (WARN_ON_ONCE(blk_rq_is_internal(rq))) { + blk_mq_end_request(rq, BLK_STS_NOTSUPP); + return; + } + blk_account_io_start(rq); /* diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index ccb895f911b1..e386c43e4d77 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -360,6 +360,7 @@ enum req_flag_bits { /* for driver use */ __REQ_DRV, __REQ_SWAP, /* swapping request. */ + __REQ_INTERNAL, /* driver-internal command */ __REQ_NR_BITS, /* stops here */ }; @@ -384,6 +385,7 @@ enum req_flag_bits { #define REQ_DRV (1ULL << __REQ_DRV) #define REQ_SWAP (1ULL << __REQ_SWAP) +#define REQ_INTERNAL (1ULL << __REQ_INTERNAL) #define REQ_FAILFAST_MASK \ (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 8fd900998b4e..d09210d4591e 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -273,6 +273,11 @@ static inline bool blk_rq_is_passthrough(struct request *rq) return blk_rq_is_scsi(rq) || blk_rq_is_private(rq); } +static inline bool blk_rq_is_internal(struct request *rq) +{ + return rq->cmd_flags & REQ_INTERNAL; +} + static inline bool bio_is_passthrough(struct bio *bio) { unsigned op = bio_op(bio);
Some drivers require to allocate requests for internal command submission. These request will never be passed through the block layer, but nevertheless require a valid tag to avoid them clashing with normal I/O commands. This patch adds a new request flag REQ_INTERNAL to mark such requests and a terminates any such commands in blk_execute_rq_nowait() with a WARN_ON_ONCE to signal such an invalid usage. Signed-off-by: Hannes Reinecke <hare@suse.de> --- block/blk-exec.c | 5 +++++ include/linux/blk_types.h | 2 ++ include/linux/blkdev.h | 5 +++++ 3 files changed, 12 insertions(+)