| Message ID | 673fc6ca8f2e761586d21c709348642113f13f86.1634676157.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block optimisation round | expand |
On Tue, Oct 19, 2021 at 10:24:22PM +0100, Pavel Begunkov wrote: > If we know that a iocb is async we can optimise bio_set_polled() a bit, > add a new helper bio_set_polled_async(). This avoids a single if. Why? I'm really worried about all these micro-optimizations that just make the code harder and harder to maintain.
On 10/20/21 07:37, Christoph Hellwig wrote: > On Tue, Oct 19, 2021 at 10:24:22PM +0100, Pavel Begunkov wrote: >> If we know that a iocb is async we can optimise bio_set_polled() a bit, >> add a new helper bio_set_polled_async(). > > This avoids a single if. Why? I'm really worried about all these > micro-optimizations that just make the code harder and harder to > maintain. Not really just one, it also moves REQ_F_NOWAIT, and alias analysis doesn't work well here, e.g. it can't conclude anything about relations b/w @iocb and @bio, those can do nothing but store/load to/from memory. Assembly related to that HIPRI path before: # block/fops.c:378: if (iocb->ki_flags & IOCB_NOWAIT) movl 32(%rbx), %eax # iocb_31(D)->ki_flags, _20 # block/fops.c:378: if (iocb->ki_flags & IOCB_NOWAIT) testb $8, %al #, _20 je .L200 #, # block/fops.c:379: bio->bi_opf |= REQ_NOWAIT; orl $2097152, 16(%r12) #, bio_40->bi_opf # block/fops.c:380: if (iocb->ki_flags & IOCB_HIPRI) { movl 32(%rbx), %eax # iocb_31(D)->ki_flags, _20 .L200: # block/fops.c:380: if (iocb->ki_flags & IOCB_HIPRI) { testb $1, %al #, _20 je .L201 #, # ./include/linux/bio.h:748: bio->bi_opf |= REQ_POLLED; movl 16(%r12), %eax # bio_40->bi_opf, _73 movl %eax, %edx # _73, tmp138 orl $16777216, %edx #, tmp138 movl %edx, 16(%r12) # tmp138, bio_40->bi_opf # ./include/linux/bio.h:749: if (!is_sync_kiocb(kiocb)) cmpq $0, 16(%rbx) #, iocb_31(D)->ki_complete je .L202 #, # ./include/linux/bio.h:750: bio->bi_opf |= REQ_NOWAIT; orl $18874368, %eax #, tmp139 movl %eax, 16(%r12) # tmp139, bio_40->bi_opf .L202: # block/fops.c:382: submit_bio(bio); movq %r12, %rdi # bio, call submit_bio # After: # block/fops.c:378: if (iocb->ki_flags & IOCB_HIPRI) { movl 32(%rbx), %eax # iocb_30(D)->ki_flags, _20 # block/fops.c:378: if (iocb->ki_flags & IOCB_HIPRI) { testb $1, %al #, _20 je .L210 #, .L213: # ./include/linux/bio.h:755: bio->bi_opf |= REQ_POLLED | REQ_NOWAIT; orl $18874368, 16(%r12) #, bio_39->bi_opf # block/fops.c:380: submit_bio(bio); movq %r12, %rdi # bio, call submit_bio #
diff --git a/block/fops.c b/block/fops.c index 0f1332374756..ee27ffbdd018 100644 --- a/block/fops.c +++ b/block/fops.c @@ -371,17 +371,17 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb, task_io_account_write(bio->bi_iter.bi_size); } - if (iocb->ki_flags & IOCB_NOWAIT) - bio->bi_opf |= REQ_NOWAIT; /* * Don't plug for HIPRI/polled IO, as those should go straight * to issue */ if (iocb->ki_flags & IOCB_HIPRI) { - bio_set_polled(bio, iocb); + bio_set_polled_async(bio, iocb); submit_bio(bio); WRITE_ONCE(iocb->private, bio); } else { + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; submit_bio(bio); } return -EIOCBQUEUED; diff --git a/include/linux/bio.h b/include/linux/bio.h index 0ab4fa2c89c3..4043e0774b89 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -743,6 +743,11 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb) bio->bi_opf |= REQ_NOWAIT; } +static inline void bio_set_polled_async(struct bio *bio, struct kiocb *kiocb) +{ + bio->bi_opf |= REQ_POLLED | REQ_NOWAIT; +} + struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp); #endif /* __LINUX_BIO_H */
If we know that a iocb is async we can optimise bio_set_polled() a bit, add a new helper bio_set_polled_async(). Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/fops.c | 6 +++--- include/linux/bio.h | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-)