| Message ID | 20191008223954.6084-1-damien.lemoal@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] block: Fix elv_support_iosched() | expand |
On 10/9/19 6:39 AM, Damien Le Moal wrote: > A BIO based request queue does not have a tag_set, which prevent testing > for the flag BLK_MQ_F_NO_SCHED indicating that the queue does not > require an elevator. This leads to an incorrect initialization of a > default elevator in some cases such as BIO based null_blk > (queue_mode == BIO) with zoned mode enabled as the default elevator in > this case is mq-deadline instead of "none". > > Fix this by testing for a NULL queue mq_ops field which indicates that > the queue is BIO based and should not have an elevator. > > Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > Changes from V1: > * Test if q->mq_ops is NULL to identify BIO based queues > > block/elevator.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/elevator.c b/block/elevator.c > index 5437059c9261..076ba7308e65 100644 > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -616,7 +616,8 @@ int elevator_switch_mq(struct request_queue *q, > > static inline bool elv_support_iosched(struct request_queue *q) > { > - if (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED)) > + if (!q->mq_ops || > + (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED))) > return false; > return true; > } > Looks good to me. Reviewed-by: Bob Liu <bob.liu@oracle.com>
On 2019/10/09 7:40, Damien Le Moal wrote: > A BIO based request queue does not have a tag_set, which prevent testing > for the flag BLK_MQ_F_NO_SCHED indicating that the queue does not > require an elevator. This leads to an incorrect initialization of a > default elevator in some cases such as BIO based null_blk > (queue_mode == BIO) with zoned mode enabled as the default elevator in > this case is mq-deadline instead of "none". > > Fix this by testing for a NULL queue mq_ops field which indicates that > the queue is BIO based and should not have an elevator. > > Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > Changes from V1: > * Test if q->mq_ops is NULL to identify BIO based queues > > block/elevator.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/elevator.c b/block/elevator.c > index 5437059c9261..076ba7308e65 100644 > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -616,7 +616,8 @@ int elevator_switch_mq(struct request_queue *q, > > static inline bool elv_support_iosched(struct request_queue *q) > { > - if (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED)) > + if (!q->mq_ops || > + (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED))) > return false; > return true; > } > Jens, Ping ? This was not in your rc3 pull request...
On 10/13/19 6:03 PM, Damien Le Moal wrote: > On 2019/10/09 7:40, Damien Le Moal wrote: >> A BIO based request queue does not have a tag_set, which prevent testing >> for the flag BLK_MQ_F_NO_SCHED indicating that the queue does not >> require an elevator. This leads to an incorrect initialization of a >> default elevator in some cases such as BIO based null_blk >> (queue_mode == BIO) with zoned mode enabled as the default elevator in >> this case is mq-deadline instead of "none". >> >> Fix this by testing for a NULL queue mq_ops field which indicates that >> the queue is BIO based and should not have an elevator. >> >> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> >> --- >> Changes from V1: >> * Test if q->mq_ops is NULL to identify BIO based queues >> >> block/elevator.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/block/elevator.c b/block/elevator.c >> index 5437059c9261..076ba7308e65 100644 >> --- a/block/elevator.c >> +++ b/block/elevator.c >> @@ -616,7 +616,8 @@ int elevator_switch_mq(struct request_queue *q, >> >> static inline bool elv_support_iosched(struct request_queue *q) >> { >> - if (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED)) >> + if (!q->mq_ops || >> + (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED))) >> return false; >> return true; >> } >> > > Jens, > > Ping ? This was not in your rc3 pull request... Yeah that missed -rc3, I've queued it up for -rc4. Thanks!
diff --git a/block/elevator.c b/block/elevator.c index 5437059c9261..076ba7308e65 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -616,7 +616,8 @@ int elevator_switch_mq(struct request_queue *q, static inline bool elv_support_iosched(struct request_queue *q) { - if (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED)) + if (!q->mq_ops || + (q->tag_set && (q->tag_set->flags & BLK_MQ_F_NO_SCHED))) return false; return true; }
A BIO based request queue does not have a tag_set, which prevent testing for the flag BLK_MQ_F_NO_SCHED indicating that the queue does not require an elevator. This leads to an incorrect initialization of a default elevator in some cases such as BIO based null_blk (queue_mode == BIO) with zoned mode enabled as the default elevator in this case is mq-deadline instead of "none". Fix this by testing for a NULL queue mq_ops field which indicates that the queue is BIO based and should not have an elevator. Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- Changes from V1: * Test if q->mq_ops is NULL to identify BIO based queues block/elevator.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)