| Message ID | 20200409170915.30570-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bfq: Waker logic tweaks for dbench performance | expand |
> Il giorno 9 apr 2020, alle ore 19:09, Jan Kara <jack@suse.cz> ha scritto: > > The check in bfq_select_queue() checking whether a waker queue should be > selected has a bug and is checking bfqq->next_rq instead of > bfqq->waker_bfqq->next_rq to verify whether the waker queue has a > request to dispatch. This often results in the condition being false > (most notably when the current queue is idling waiting for next request) > and thus the waker queue logic is ineffective. Fix the condition. > Hi Jan, my huge delay causes problems again, because a student of mine already made this same fix a few months ago. But I did not send it out yet, for lack of time. If ok for you, we could go for a common commit with two authors (I seem to remember it is feasible). Thanks. Paolo > Signed-off-by: Jan Kara <jack@suse.cz> > --- > block/bfq-iosched.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > index 78ba57efd16b..18f85d474c9c 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -4498,7 +4498,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd) > bfqq = bfqq->bic->bfqq[0]; > else if (bfq_bfqq_has_waker(bfqq) && > bfq_bfqq_busy(bfqq->waker_bfqq) && > - bfqq->next_rq && > + bfqq->waker_bfqq->next_rq && > bfq_serv_to_charge(bfqq->waker_bfqq->next_rq, > bfqq->waker_bfqq) <= > bfq_bfqq_budget_left(bfqq->waker_bfqq) > -- > 2.16.4 >
On Sun 10-01-21 10:20:22, Paolo Valente wrote: > > > > Il giorno 9 apr 2020, alle ore 19:09, Jan Kara <jack@suse.cz> ha scritto: > > > > The check in bfq_select_queue() checking whether a waker queue should be > > selected has a bug and is checking bfqq->next_rq instead of > > bfqq->waker_bfqq->next_rq to verify whether the waker queue has a > > request to dispatch. This often results in the condition being false > > (most notably when the current queue is idling waiting for next request) > > and thus the waker queue logic is ineffective. Fix the condition. > > > > Hi Jan, > my huge delay causes problems again, because a student of mine already > made this same fix a few months ago. But I did not send it out yet, > for lack of time. If ok for you, we could go for a common commit with > two authors (I seem to remember it is feasible). No problem for me. Or just give the student a credit instead of me. A commit in the kernel is likely more interesting for him than for me ;) Just reply to the v2 series I've sent today (you should be on CC) so that Jens knows the author should be changed. Honza
On Wed 13-01-21 14:09:33, Jan Kara wrote: > On Sun 10-01-21 10:20:22, Paolo Valente wrote: > > > > > > > Il giorno 9 apr 2020, alle ore 19:09, Jan Kara <jack@suse.cz> ha scritto: > > > > > > The check in bfq_select_queue() checking whether a waker queue should be > > > selected has a bug and is checking bfqq->next_rq instead of > > > bfqq->waker_bfqq->next_rq to verify whether the waker queue has a > > > request to dispatch. This often results in the condition being false > > > (most notably when the current queue is idling waiting for next request) > > > and thus the waker queue logic is ineffective. Fix the condition. > > > > > > > Hi Jan, > > my huge delay causes problems again, because a student of mine already > > made this same fix a few months ago. But I did not send it out yet, > > for lack of time. If ok for you, we could go for a common commit with > > two authors (I seem to remember it is feasible). > > No problem for me. Or just give the student a credit instead of me. A > commit in the kernel is likely more interesting for him than for me ;) Just > reply to the v2 series I've sent today (you should be on CC) so that Jens > knows the author should be changed. Oh, this is a different patch than I thought. I didn't resubmit this one. Nevertheless "I don't care" still holds :). Just submit whatever you find fit. Honza
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 78ba57efd16b..18f85d474c9c 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -4498,7 +4498,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd) bfqq = bfqq->bic->bfqq[0]; else if (bfq_bfqq_has_waker(bfqq) && bfq_bfqq_busy(bfqq->waker_bfqq) && - bfqq->next_rq && + bfqq->waker_bfqq->next_rq && bfq_serv_to_charge(bfqq->waker_bfqq->next_rq, bfqq->waker_bfqq) <= bfq_bfqq_budget_left(bfqq->waker_bfqq)
The check in bfq_select_queue() checking whether a waker queue should be selected has a bug and is checking bfqq->next_rq instead of bfqq->waker_bfqq->next_rq to verify whether the waker queue has a request to dispatch. This often results in the condition being false (most notably when the current queue is idling waiting for next request) and thus the waker queue logic is ineffective. Fix the condition. Signed-off-by: Jan Kara <jack@suse.cz> --- block/bfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)