| Message ID | a21e91fff1abe201ebbc010d596b034f7b5123de.1574974577.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-mq: optimise plugging | expand |
On 28.11.19 г. 23:11 ч., Pavel Begunkov wrote: > Check "!=" in multi-layer comparisons. The same memory usage, fewer > instructions, and 2 from 4 jumps are replaced with SETcc. > > Note, that list_sort() doesn't differ 0 and <0. > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> My first reaction was this is wrong since you no longer return negative values. But then I looked into list_sort/merge and this branch 'if (cmp(priv, a, b) <= 0) {' clearly shows this is correct. So : Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > block/blk-mq.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 323c9cb28066..f32a3cfdd34e 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1668,14 +1668,10 @@ static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b) > struct request *rqa = container_of(a, struct request, queuelist); > struct request *rqb = container_of(b, struct request, queuelist); > > - if (rqa->mq_ctx < rqb->mq_ctx) > - return -1; > - else if (rqa->mq_ctx > rqb->mq_ctx) > - return 1; > - else if (rqa->mq_hctx < rqb->mq_hctx) > - return -1; > - else if (rqa->mq_hctx > rqb->mq_hctx) > - return 1; > + if (rqa->mq_ctx != rqb->mq_ctx) > + return rqa->mq_ctx > rqb->mq_ctx; > + if (rqa->mq_hctx != rqb->mq_hctx) > + return rqa->mq_hctx > rqb->mq_hctx; > > return blk_rq_pos(rqa) > blk_rq_pos(rqb); > } >
On 11/29/2019 11:28 AM, Nikolay Borisov wrote: > On 28.11.19 г. 23:11 ч., Pavel Begunkov wrote: >> Check "!=" in multi-layer comparisons. The same memory usage, fewer >> instructions, and 2 from 4 jumps are replaced with SETcc. >> >> Note, that list_sort() doesn't differ 0 and <0. >> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > > My first reaction was this is wrong since you no longer return negative > values. But then I looked into list_sort/merge and this branch > 'if (cmp(priv, a, b) <= 0) {' clearly shows this is correct. Yes, that's why there is a note in the patch description. The same is told by list_sort() description. > > So : > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> Thanks for taking a look > >> --- >> block/blk-mq.c | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 323c9cb28066..f32a3cfdd34e 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -1668,14 +1668,10 @@ static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b) >> struct request *rqa = container_of(a, struct request, queuelist); >> struct request *rqb = container_of(b, struct request, queuelist); >> >> - if (rqa->mq_ctx < rqb->mq_ctx) >> - return -1; >> - else if (rqa->mq_ctx > rqb->mq_ctx) >> - return 1; >> - else if (rqa->mq_hctx < rqb->mq_hctx) >> - return -1; >> - else if (rqa->mq_hctx > rqb->mq_hctx) >> - return 1; >> + if (rqa->mq_ctx != rqb->mq_ctx) >> + return rqa->mq_ctx > rqb->mq_ctx; >> + if (rqa->mq_hctx != rqb->mq_hctx) >> + return rqa->mq_hctx > rqb->mq_hctx; >> >> return blk_rq_pos(rqa) > blk_rq_pos(rqb); >> } >>
On 29.11.19 г. 11:45 ч., Pavel Begunkov wrote: > On 11/29/2019 11:28 AM, Nikolay Borisov wrote: >> On 28.11.19 г. 23:11 ч., Pavel Begunkov wrote: >>> Check "!=" in multi-layer comparisons. The same memory usage, fewer >>> instructions, and 2 from 4 jumps are replaced with SETcc. >>> >>> Note, that list_sort() doesn't differ 0 and <0. >>> >>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> >> My first reaction was this is wrong since you no longer return negative >> values. But then I looked into list_sort/merge and this branch >> 'if (cmp(priv, a, b) <= 0) {' clearly shows this is correct. > > Yes, that's why there is a note in the patch description. The same is > told by list_sort() description. And of course I have missed your remark in the changelog :) > >> >> So : >> >> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > > Thanks for taking a look > >> >>> --- >>> block/blk-mq.c | 12 ++++-------- >>> 1 file changed, 4 insertions(+), 8 deletions(-) >>> >>> diff --git a/block/blk-mq.c b/block/blk-mq.c >>> index 323c9cb28066..f32a3cfdd34e 100644 >>> --- a/block/blk-mq.c >>> +++ b/block/blk-mq.c >>> @@ -1668,14 +1668,10 @@ static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b) >>> struct request *rqa = container_of(a, struct request, queuelist); >>> struct request *rqb = container_of(b, struct request, queuelist); >>> >>> - if (rqa->mq_ctx < rqb->mq_ctx) >>> - return -1; >>> - else if (rqa->mq_ctx > rqb->mq_ctx) >>> - return 1; >>> - else if (rqa->mq_hctx < rqb->mq_hctx) >>> - return -1; >>> - else if (rqa->mq_hctx > rqb->mq_hctx) >>> - return 1; >>> + if (rqa->mq_ctx != rqb->mq_ctx) >>> + return rqa->mq_ctx > rqb->mq_ctx; >>> + if (rqa->mq_hctx != rqb->mq_hctx) >>> + return rqa->mq_hctx > rqb->mq_hctx; >>> >>> return blk_rq_pos(rqa) > blk_rq_pos(rqb); >>> } >>> >
diff --git a/block/blk-mq.c b/block/blk-mq.c index 323c9cb28066..f32a3cfdd34e 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1668,14 +1668,10 @@ static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b) struct request *rqa = container_of(a, struct request, queuelist); struct request *rqb = container_of(b, struct request, queuelist); - if (rqa->mq_ctx < rqb->mq_ctx) - return -1; - else if (rqa->mq_ctx > rqb->mq_ctx) - return 1; - else if (rqa->mq_hctx < rqb->mq_hctx) - return -1; - else if (rqa->mq_hctx > rqb->mq_hctx) - return 1; + if (rqa->mq_ctx != rqb->mq_ctx) + return rqa->mq_ctx > rqb->mq_ctx; + if (rqa->mq_hctx != rqb->mq_hctx) + return rqa->mq_hctx > rqb->mq_hctx; return blk_rq_pos(rqa) > blk_rq_pos(rqb); }
Check "!=" in multi-layer comparisons. The same memory usage, fewer instructions, and 2 from 4 jumps are replaced with SETcc. Note, that list_sort() doesn't differ 0 and <0. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/blk-mq.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)