| Message ID | c01c0d2c4d626eb1b63b196d98375a7e89d50db3.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:14PM +0100, Pavel Begunkov wrote: > Inline BIO_NO_PAGE_REF check of bio_release_pages() to avoid function > call. Fine with me, but it seems like we're really getting into benchmarketing at some point..
On 10/20/21 07:13, Christoph Hellwig wrote: > On Tue, Oct 19, 2021 at 10:24:14PM +0100, Pavel Begunkov wrote: >> Inline BIO_NO_PAGE_REF check of bio_release_pages() to avoid function >> call. > > Fine with me, but it seems like we're really getting into benchmarketing > at some point.. Well, certainly don't want to get to that point, but at least this one is in io_uring's hot path, i.e. fixed buffers.
On 10/20/21 12:13 AM, Christoph Hellwig wrote: > On Tue, Oct 19, 2021 at 10:24:14PM +0100, Pavel Begunkov wrote: >> Inline BIO_NO_PAGE_REF check of bio_release_pages() to avoid function >> call. > > Fine with me, but it seems like we're really getting into benchmarketing > at some point.. I just want to be very clear that neither mine nor Pavel's work is trying to get into benchmarketing. There are very tangible performance benefits, and they are backed up by code analysis and testing. That said, the point is of course not to make this any harder to follow or maintain, but we are doing suboptimal things in various places and those should get cleaned up, particularly when they impact performance. Are some a big eager? Perhaps, but let's not that that cloud the perception of the effort as a whole. The fact that 5.15-rc6 will do ~5.5M/core and post these optimizations we'll be at 9M/core is a solid 60%+ improvement. That's not benchmarketing, that's just making the stack more efficient.
On Wed, Oct 20, 2021 at 08:15:25AM -0600, Jens Axboe wrote: > I just want to be very clear that neither mine nor Pavel's work is > trying to get into benchmarketing. There are very tangible performance > benefits, and they are backed up by code analysis and testing. That > said, the point is of course not to make this any harder to follow or > maintain, but we are doing suboptimal things in various places and those > should get cleaned up, particularly when they impact performance. Are > some a big eager? Perhaps, but let's not that that cloud the perception > of the effort as a whole. A lot of it seems generally useful. But optimizing for BIO_NO_PAGE_REF is really special. Besides a few in-kernel special cases this is all about the io_uring pre-registered buffers, which are very much a special case and not a normal workload at all. In this case it is just an extra conditional moved into a few callers so I think we're ok, but I'm really worried about optimizing this benchmark fast path over the code everyone actually uses.
diff --git a/block/bio.c b/block/bio.c index 4f397ba47db5..46a87c72d2b4 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1033,21 +1033,18 @@ int bio_add_page(struct bio *bio, struct page *page, } EXPORT_SYMBOL(bio_add_page); -void bio_release_pages(struct bio *bio, bool mark_dirty) +void __bio_release_pages(struct bio *bio, bool mark_dirty) { struct bvec_iter_all iter_all; struct bio_vec *bvec; - if (bio_flagged(bio, BIO_NO_PAGE_REF)) - return; - bio_for_each_segment_all(bvec, bio, iter_all) { if (mark_dirty && !PageCompound(bvec->bv_page)) set_page_dirty_lock(bvec->bv_page); put_page(bvec->bv_page); } } -EXPORT_SYMBOL_GPL(bio_release_pages); +EXPORT_SYMBOL_GPL(__bio_release_pages); static void __bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter) { diff --git a/include/linux/bio.h b/include/linux/bio.h index b12453d7b8a8..c88700d1bdc3 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -417,7 +417,7 @@ int bio_add_zone_append_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); -void bio_release_pages(struct bio *bio, bool mark_dirty); +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); @@ -428,6 +428,12 @@ extern void bio_free_pages(struct bio *bio); void guard_bio_eod(struct bio *bio); void zero_fill_bio(struct bio *bio); +static inline void bio_release_pages(struct bio *bio, bool mark_dirty) +{ + if (!bio_flagged(bio, BIO_NO_PAGE_REF)) + __bio_release_pages(bio, mark_dirty); +} + extern const char *bio_devname(struct bio *bio, char *buffer); #define bio_dev(bio) \
Inline BIO_NO_PAGE_REF check of bio_release_pages() to avoid function call. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/bio.c | 7 ++----- include/linux/bio.h | 8 +++++++- 2 files changed, 9 insertions(+), 6 deletions(-)