optimise iov_iter
Subsystems (5)
v2 cover optimise iov_iter
The first patch optimises iov_iter_npages() for the bvec case, and the second helps code generation to kill unreachable code.
v2: same code, stylistic message changes
+ Reviewed-by
Pavel Begunkov (2):
iov_iter: optimise iov_iter_npages for bvec
iov_iter: optimise iter type checking
include/linux/uio.h | 10 +++++----- lib/iov_iter.c | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-)
Signature · 1 line
2.24.0
PATCH v2 1/2 iov_iter: optimise iov_iter_npages for bvec
The block layer spends quite a while in iov_iter_npages(), but for the bvec case the number of pages is already known and stored in iter->nr_segs, so it can be returned immediately as an optimisation
Perf for an io_uring benchmark with registered buffers (i.e. bvec) shows ~1.5-2.0% total cycle count spent in iov_iter_npages(), that's dropped by this patch to ~0.2%.
--- lib/iov_iter.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
lib/iov_iter.c +5 -5
@@ -1594,6 +1594,8 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages)return 0 ;if (unlikely (iov_iter_is_discard (i)))return 0 ; +if (unlikely (iov_iter_is_bvec (i))) +return min_t (int, i->nr_segs, maxpages);if (unlikely (iov_iter_is_pipe (i))) {struct pipe_inode_info *pipe = i->pipe;
@@ -1614,11 +1616,9 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages) - p /PAGE_SIZE ;if (npages >= maxpages)return maxpages; - 0;}),({ - npages++; -if (npages >= maxpages) -return maxpages; - }),({ + 0;}), + 0 /* bvecs are handled above */ + ,({unsigned long p = (unsigned long )v.iov_base; npages +=DIV_ROUND_UP (p + v.iov_len,PAGE_SIZE ) - p /PAGE_SIZE ;
Signature · 1 line
2.24.0
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisation
Er ... no, it doesn't. nr_segs is the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.
Ah, really, missed min() with PAGE_SIZE in bvec_iter_len(), then it's a stupid statement. Thanks!
Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 6 lines · Pavel Begunkov
Quoted 5 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisation
Quoted 8 lines
Quoted 2 lines
Er ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
__bio_try_merge_page() will create multipage BIOs, and that's called from a number of places including bio_try_merge_hw_seg(), bio_add_page(), and __bio_iov_iter_get_pages()
so ... yeah, it's used a lot.
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 13 lines · Matthew Wilcox
Quoted 12 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!
Quoted 6 lines
Quoted 2 lines
Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()
I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but bvec.h can be nicely optimised without it.
Quoted 3 lines
so ... yeah, it's used a lot.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 17 lines · Pavel Begunkov
Quoted 16 lines · Matthew Wilcox
Quoted 14 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
Quoted 8 lines
Quoted 3 lines
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but
bvec.hcan be nicely optimised without it.
Typically when you're allocating pages for the page cache, they'll get allocated in order and then you'll read or write them in order, so yes, it ends up triggering quite a lot. There was once a bug in the page allocator which caused them to get allocated in reverse order and it was a noticable performance hit (this was 15-20 years ago).
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 27 lines · Matthew Wilcox
Quoted 17 lines · Pavel Begunkov
Quoted 16 lines · Matthew Wilcox
Quoted 14 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
Quoted 8 lines
Quoted 3 lines
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but
bvec.hcan be nicely optimised without it.
Quoted 5 lines
Typically when you're allocating pages for the page cache, they'll get allocated in order and then you'll read or write them in order, so yes, it ends up triggering quite a lot. There was once a bug in the page allocator which caused them to get allocated in reverse order and it was a noticable performance hit (this was 15-20 years ago).
I see, thanks for a bit of insight
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
In reply to Matthew WilcoxQuoted 27 lines · Matthew Wilcox
Quoted 17 lines · Pavel Begunkov
Quoted 16 lines · Matthew Wilcox
Quoted 14 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
Quoted 8 lines
Quoted 3 lines
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but
bvec.hcan be nicely optimised without it.
Quoted 5 lines
Typically when you're allocating pages for the page cache, they'll get allocated in order and then you'll read or write them in order, so yes, it ends up triggering quite a lot. There was once a bug in the page allocator which caused them to get allocated in reverse order and it was a noticable performance hit (this was 15-20 years ago).
hugepage use cases can benefit much from this way too.
Thanks,
Ming
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 28 lines · Ming Lei
Quoted 27 lines · Matthew Wilcox
Quoted 25 lines · Pavel Begunkov
Quoted 13 lines · Matthew Wilcox
Quoted 12 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Quoted 6 lines
Quoted 2 lines
Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but
bvec.hcan be nicely optimised without it.
Quoted 7 lines
Quoted 5 lines
Typically when you're allocating pages for the page cache, they'll get allocated in order and then you'll read or write them in order, so yes, it ends up triggering quite a lot. There was once a bug in the page allocator which caused them to get allocated in reverse order and it was a noticable performance hit (this was 15-20 years ago).
hugepage use cases can benefit much from this way too.
This didn't yield any considerable boost for me though. 1.5% -> 1.3% for 1 page reads. I'll send it anyway though because there are cases that can benefit, e.g. as Ming mentioned.
Ming would you want to send the patch yourself? After all you did post it first.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 35 lines · Pavel Begunkov
Quoted 34 lines · Ming Lei
Quoted 32 lines · Matthew Wilcox
Quoted 17 lines · Pavel Begunkov
Quoted 16 lines · Matthew Wilcox
Quoted 14 lines · Pavel Begunkov
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationEr ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!Are there many users of that? All these iterators are a huge burden, just to count one 4KB page in bvec it takes 2% of CPU time for me.
Quoted 8 lines
Quoted 3 lines
__bio_try_merge_page()will create multipage BIOs, and that's called from a number of places includingbio_try_merge_hw_seg(),bio_add_page(), and__bio_iov_iter_get_pages()I get it that there are a lot of places, more interesting how often it's actually triggered and if that's performance critical for anybody. Not like I'm going to change it, just out of curiosity, but
bvec.hcan be nicely optimised without it.Typically when you're allocating pages for the page cache, they'll get allocated in order and then you'll read or write them in order, so yes, it ends up triggering quite a lot. There was once a bug in the page allocator which caused them to get allocated in reverse order and it was a noticable performance hit (this was 15-20 years ago).
Quoted 5 lines
Quoted 1 line
hugepage use cases can benefit much from this way too.
This didn't yield any considerable boost for me though. 1.5% -> 1.3% for 1 page reads. I'll send it anyway though because there are cases that can benefit, e.g. as Ming mentioned.
And yeah, it just shifts my attention for optimisation to its callers, e.g. blkdev_direct_IO.
Quoted 3 lines
Ming would you want to send the patch yourself? After all you did post it first.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
In reply to Pavel BegunkovQuoted 6 lines · Pavel Begunkov
Quoted 5 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisation
Quoted 6 lines
Quoted 2 lines
Er ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!
iov_iter_npages(bvec) still can be improved a bit by the following way:
lib/iov_iter.c +11 -5
@@ -1608,17 +1608,23 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages) npages =pipe_space_for_user (iter_head, pipe->tail, pipe);if (npages >= maxpages)return maxpages; + } else if (iov_iter_is_bvec(i)) { +unsigned idx, offset = i->iov_offset; + +for (idx =0 ; idx < i->nr_segs; idx++) { + npages +=DIV_ROUND_UP (i->bvec[idx].bv_len - offset, +PAGE_SIZE ); + offset =0 ; + } +if (npages >= maxpages) +return maxpages; } else iterate_all_kinds(i, size, v, ({unsigned long p = (unsigned long )v.iov_base; npages +=DIV_ROUND_UP (p + v.iov_len,PAGE_SIZE ) - p /PAGE_SIZE ;if (npages >= maxpages)return maxpages; - 0;}),({ - npages++; - if (npages >= maxpages) - return maxpages; - }),({ + 0;}),0,({ unsigned long p = (unsigned long)v.iov_base; npages +=DIV_ROUND_UP (p + v.iov_len,PAGE_SIZE ) - p /PAGE_SIZE ;
Signature · 1 line
Ming
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 7 lines · Ming Lei
Quoted 6 lines · Pavel Begunkov
Quoted 5 lines · Matthew Wilcox
Quoted 3 lines · Pavel Begunkov
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisation
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Er ... no, it doesn't.
nr_segsis the number of bvecs. Each bvec can store up to 4GB of contiguous physical memory.Ah, really, missed
min()withPAGE_SIZEinbvec_iter_len(), then it's a stupid statement. Thanks!
iov_iter_npages(bvec)still can be improved a bit by the following way:
Yep, was doing exactly that, +a couple of other places that are in my way.
Quoted 35 lines
lib/iov_iter.c +11
-5@@ -1608,17 +1608,23 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages) npages =pipe_space_for_user (iter_head, pipe->tail, pipe);if (npages >= maxpages)return maxpages; + } else if (iov_iter_is_bvec(i)) { +unsigned idx, offset = i->iov_offset; + +for (idx =0 ; idx < i->nr_segs; idx++) { + npages +=DIV_ROUND_UP (i->bvec[idx].bv_len - offset, +PAGE_SIZE ); + offset =0 ; + } +if (npages >= maxpages) +return maxpages; } else iterate_all_kinds(i, size, v, ({unsigned long p = (unsigned long )v.iov_base; npages +=DIV_ROUND_UP (p + v.iov_len,PAGE_SIZE ) - p /PAGE_SIZE ;if (npages >= maxpages)return maxpages; - 0;}),({ - npages++; - if (npages >= maxpages) - return maxpages; - }),({ + 0;}),0,({ unsigned long p = (unsigned long)v.iov_base; npages +=DIV_ROUND_UP (p + v.iov_len,PAGE_SIZE ) - p /PAGE_SIZE ;
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Are you optimising the right thing here? Assuming you're looking at the one in do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from the iov_iter into the bio rather than calling dio_bio_add_page() for each page?
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 10 lines · Matthew Wilcox
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Are you optimising the right thing here? Assuming you're looking at the one in
do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from theiov_iterinto the bio rather than callingdio_bio_add_page()for each page?
Which is most effectively done by stopping to to use *blockdev_direct_IO and switching to iomap instead :)
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 7 lines · Christoph Hellwig
Quoted 6 lines · Matthew Wilcox
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Quoted 7 lines
Quoted 4 lines
Are you optimising the right thing here? Assuming you're looking at the one in
do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from theiov_iterinto the bio rather than callingdio_bio_add_page()for each page?Which is most effectively done by stopping to to use
*blockdev_direct_IOand switching to iomap instead :)
But iomap still calls iov_iter_npages(). So maybe we need something like this ...
fs/iomap/direct-io.c +2 -2
@@ -250,7 +250,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, orig_count =iov_iter_count (dio->submit.iter);iov_iter_truncate (dio->submit.iter, length); - nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);if (nr_pages <=0 ) { ret = nr_pages; goto out;
@@ -308,7 +308,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, dio->size += n; copied += n; - nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);iomap_dio_submit_bio (dio, iomap, bio, pos); pos += n; }while (nr_pages);
include/linux/bio.h +11 -0
@@ -10,6 +10,7 @@#include <linux/ioprio.h> /* struct bio, bio_vec and BIO_* flags are defined in blk_types.h */ #include <linux/blk_types.h> +#include <linux/uio.h> #define BIO_DEBUG
@@ -447,6 +448,16 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page,void __bio_add_page (struct bio *bio,struct page *page,unsigned int len,unsigned int off);int bio_iov_iter_get_pages (struct bio *bio,struct iov_iter *iter); + +static inline int bio_iov_iter_npages (const struct iov_iter *i) +{ +if (!iov_iter_count (i)) +return 0 ; +if (iov_iter_is_bvec (i)) +return 1 ; +return iov_iter_npages (i,BIO_MAX_PAGES ); +} +void bio_release_pages (struct bio *bio,bool mark_dirty);extern void bio_set_pages_dirty (struct bio *bio);extern void bio_check_pages_dirty (struct bio *bio);
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 13 lines · Matthew Wilcox
Quoted 12 lines · Christoph Hellwig
Quoted 10 lines · Matthew Wilcox
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Are you optimising the right thing here? Assuming you're looking at the one in
do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from theiov_iterinto the bio rather than callingdio_bio_add_page()for each page?
Quoted 5 lines
Quoted 2 lines
Which is most effectively done by stopping to to use
*blockdev_direct_IOand switching to iomap instead :)But iomap still calls
iov_iter_npages(). So maybe we need something like this ...
Yep, all that are not mutually exclusive optimisations. Why return 1? It seems to be used later in bio_alloc(nr_pages)
Quoted 53 lines
fs/iomap/direct-io.c +2
-2@@ -250,7 +250,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, orig_count =iov_iter_count (dio->submit.iter);iov_iter_truncate (dio->submit.iter, length); - nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);if (nr_pages <=0 ) { ret = nr_pages; goto out;@@ -308,7 +308,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, dio->size += n; copied += n; - nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);iomap_dio_submit_bio (dio, iomap, bio, pos); pos += n; }while (nr_pages);include/linux/bio.h +11
-0@@ -10,6 +10,7 @@#include <linux/ioprio.h> /* struct bio, bio_vec and BIO_* flags are defined in blk_types.h */ #include <linux/blk_types.h> +#include <linux/uio.h> #define BIO_DEBUG @@ -447,6 +448,16 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page,void __bio_add_page (struct bio *bio,struct page *page,unsigned int len,unsigned int off);int bio_iov_iter_get_pages (struct bio *bio,struct iov_iter *iter); + +static inline int bio_iov_iter_npages (const struct iov_iter *i) +{ +if (!iov_iter_count (i)) +return 0 ; +if (iov_iter_is_bvec (i)) +return 1 ; +return iov_iter_npages (i,BIO_MAX_PAGES ); +} +void bio_release_pages (struct bio *bio,bool mark_dirty);extern void bio_set_pages_dirty (struct bio *bio);extern void bio_check_pages_dirty (struct bio *bio);
Signature · 1 line
Pavel Begunkov
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
Quoted 14 lines · Pavel Begunkov
Quoted 13 lines · Matthew Wilcox
Quoted 12 lines · Christoph Hellwig
Quoted 10 lines · Matthew Wilcox
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Are you optimising the right thing here? Assuming you're looking at the one in
do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from theiov_iterinto the bio rather than callingdio_bio_add_page()for each page?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Which is most effectively done by stopping to to use
*blockdev_direct_IOand switching to iomap instead :)But iomap still calls
iov_iter_npages(). So maybe we need something like this ...Yep, all that are not mutually exclusive optimisations. Why
return 1? It seems to be used later inbio_alloc(nr_pages)
because 0 means "no pages". It does no harm to allocate one biovec that we then don't use.
Quoted 3 lines
fs/iomap/direct-io.c iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, · line 254
Quoted 3 lines
fs/iomap/direct-io.c +1
-1- nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);if (nr_pages <=0 ) {
^^^^^^^^^^^^^
Quoted 5 lines
fs/iomap/direct-io.c iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, · line 314
Quoted 5 lines
fs/iomap/direct-io.c +1
-1- nr_pages =iov_iter_npages (dio->submit.iter,BIO_MAX_PAGES ); + nr_pages =bio_iov_iter_npages (dio->submit.iter);iomap_dio_submit_bio (dio, iomap, bio, pos); pos += n; }while (nr_pages);
^^^^^^^^
Re: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
In reply to Matthew WilcoxQuoted 10 lines · Matthew Wilcox
Quoted 4 lines · Pavel Begunkov
Quoted 1 line · Ming Lei
iov_iter_npages(bvec)still can be improved a bit by the following way:Yep, was doing exactly that, +a couple of other places that are in my way.
Are you optimising the right thing here? Assuming you're looking at the one in
do_blockdev_direct_IO(), wouldn't we be better off figuring out how to copy the bvecs directly from theiov_iterinto the bio rather than callingdio_bio_add_page()for each page?
Ha, you got me, *add_page() was that "couple of others". It shows up much more, but iov_iter_npages() just looked simple enough to do first.
Signature · 1 line
Pavel Begunkov
RE: [PATCH v2 1/2] iov_iter: optimise iov_iter_npages for bvec
In reply to Pavel BegunkovFrom: Pavel Begunkov
Quoted 21 lines
Sent: 19 November 2020 23:25
The block layer spends quite a while in
iov_iter_npages(), but for the bvec case the number of pages is already known and stored initer->nr_segs, so it can be returned immediately as an optimisationPerf for an
io_uringbenchmark with registered buffers (i.e. bvec) shows ~1.5-2.0% total cycle count spent iniov_iter_npages(), that's dropped by this patch to ~0.2%.Reviewed-by Jens Axboe <[email protected]>Signed-off-by Pavel Begunkov <[email protected]>--- lib/iov_iter.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)lib/iov_iter.c +0
-0
lib/iov_iter.c int iov_iter_npages(const struct iov_iter *i, int maxpages) · line 1600
Quoted 7 lines
lib/iov_iter.c +2
-0@@ -1594,6 +1594,8 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages)return 0 ;if (unlikely (iov_iter_is_discard (i)))return 0 ; +if (unlikely (iov_iter_is_bvec (i))) +return min_t (int, i->nr_segs, maxpages);if (unlikely (iov_iter_is_pipe (i))) {
Is it worth putting an extra condition around these three 'unlikely' cases. ie:
if (unlikely((iov_iter_type(i) & (ITER_DISCARD | ITER_BVEC | ITER_PIPE)) {
if (iov_iter_is_discard(i))
return 0;
if (iov_iter_is_bvec(i))
return min_t(int, i->nr_segs, maxpages);
/* Must be ITER_PIPE */
David-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
PATCH v2 2/2 iov_iter: optimise iter type checking
In reply to Pavel BegunkovThe problem here is that iov_iter_is_*() helpers check types for equality, but all iterate_* helpers do bitwise ands. This confuses compilers, so even if some cases were handled separately with iov_iter_is_*(), corresponding ifs in iterate*() right after are not eliminated.
E.g. iov_iter_npages() first handles bvecs and discards, but iterate_all_kinds() still checks those iter types and generates actually unreachable code including iter init, for(), etc.
size lib/iov_iter.o
before:
text data bss dec hex filename
24409 805 0 25214 627e lib/iov_iter.o
after:
text data bss dec hex filename
23785 793 0 24578 6002 lib/iov_iter.o
Most of it is ifs that are executed but never true.
--- include/linux/uio.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
include/linux/uio.h +5 -5
@@ -57,27 +57,27 @@ static inline enum iter_type iov_iter_type(const struct iov_iter *i)static inline bool iter_is_iovec (const struct iov_iter *i) { -return iov_iter_type (i) ==ITER_IOVEC ; +return iov_iter_type (i) &ITER_IOVEC ; }static inline bool iov_iter_is_kvec (const struct iov_iter *i) { -return iov_iter_type (i) ==ITER_KVEC ; +return iov_iter_type (i) &ITER_KVEC ; }static inline bool iov_iter_is_bvec (const struct iov_iter *i) { -return iov_iter_type (i) ==ITER_BVEC ; +return iov_iter_type (i) &ITER_BVEC ; }static inline bool iov_iter_is_pipe (const struct iov_iter *i) { -return iov_iter_type (i) ==ITER_PIPE ; +return iov_iter_type (i) &ITER_PIPE ; }static inline bool iov_iter_is_discard (const struct iov_iter *i) { -return iov_iter_type (i) ==ITER_DISCARD ; +return iov_iter_type (i) &ITER_DISCARD ; }static inline unsigned char iov_iter_rw (const struct iov_iter *i)
Signature · 1 line
2.24.0