2021-10-14 14:06:18

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 0/5] cache request_queue pointer

Cache request_queue in bdev and replace two derefs in
bdev->bd_disk->queue with bdev->bd_queue. Benchmarking
with nullblk gave me around +1% to peak perf.

All patches are self contained and don't rely on others from
the set including 1/5 and can be taken separately. And some
changes go in separate patches to minimise conflicts. When
we agree on the approach, I'll send the rest converting some
other spots out of block.

note: based on for-5.16/block-io_uring

Pavel Begunkov (5):
block: cache request queue in bdev
block: use bdev_get_queue() in bdev.c
block: use bdev_get_queue() in bio.c
block: use bdev_get_queue() in blk-core.c
block: convert the rest of block to bdev_get_queue

block/bdev.c | 9 +++++----
block/bio-integrity.c | 2 +-
block/bio.c | 10 +++++-----
block/blk-cgroup.c | 16 ++++++++--------
block/blk-core.c | 10 +++++-----
block/blk-crypto.c | 2 +-
block/blk-iocost.c | 12 ++++++------
block/blk-merge.c | 2 +-
block/blk-mq.c | 2 +-
block/blk-throttle.c | 2 +-
block/genhd.c | 8 +++++---
block/partitions/core.c | 4 ++--
include/linux/blk_types.h | 1 +
include/linux/blkdev.h | 2 +-
14 files changed, 43 insertions(+), 39 deletions(-)

--
2.33.0


2021-10-14 14:06:59

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 4/5] block: use bdev_get_queue() in blk-core.c

Convert bdev->bd_disk->queue to bdev_get_queue(), it's uses a cached
queue pointer and so is fater.

Signed-off-by: Pavel Begunkov <[email protected]>
---
block/blk-core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1ad2528680ea..f928990a1341 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -472,7 +472,7 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)

static inline int bio_queue_enter(struct bio *bio)
{
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
bool nowait = bio->bi_opf & REQ_NOWAIT;
int ret;

@@ -782,7 +782,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
static noinline_for_stack bool submit_bio_checks(struct bio *bio)
{
struct block_device *bdev = bio->bi_bdev;
- struct request_queue *q = bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bdev);
blk_status_t status = BLK_STS_IOERR;
struct blk_plug *plug;

@@ -940,7 +940,7 @@ static void __submit_bio_noacct(struct bio *bio)
current->bio_list = bio_list_on_stack;

do {
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
struct bio_list lower, same;

if (unlikely(bio_queue_enter(bio) != 0))
@@ -961,7 +961,7 @@ static void __submit_bio_noacct(struct bio *bio)
bio_list_init(&lower);
bio_list_init(&same);
while ((bio = bio_list_pop(&bio_list_on_stack[0])) != NULL)
- if (q == bio->bi_bdev->bd_disk->queue)
+ if (q == bdev_get_queue(bio->bi_bdev))
bio_list_add(&same, bio);
else
bio_list_add(&lower, bio);
@@ -1056,7 +1056,7 @@ void submit_bio(struct bio *bio)

if (unlikely(bio_op(bio) == REQ_OP_WRITE_SAME))
count = queue_logical_block_size(
- bio->bi_bdev->bd_disk->queue) >> 9;
+ bdev_get_queue(bio->bi_bdev)) >> 9;
else
count = bio_sectors(bio);

--
2.33.0

2021-10-14 14:07:18

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 3/5] block: use bdev_get_queue() in bio.c

Convert bdev->bd_disk->queue to bdev_get_queue(), it's uses a cached
queue pointer and so is fater.

Signed-off-by: Pavel Begunkov <[email protected]>
---
block/bio.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 7377fc93606a..fb3704a61e07 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -910,7 +910,7 @@ EXPORT_SYMBOL(bio_add_pc_page);
int bio_add_zone_append_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
bool same_page = false;

if (WARN_ON_ONCE(bio_op(bio) != REQ_OP_ZONE_APPEND))
@@ -1054,7 +1054,7 @@ static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)

static int bio_iov_bvec_set_append(struct bio *bio, struct iov_iter *iter)
{
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
struct iov_iter i = *iter;

iov_iter_truncate(&i, queue_max_zone_append_sectors(q) << 9);
@@ -1132,7 +1132,7 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
{
unsigned short nr_pages = bio->bi_max_vecs - bio->bi_vcnt;
unsigned short entries_left = bio->bi_max_vecs - bio->bi_vcnt;
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
unsigned int max_append_sectors = queue_max_zone_append_sectors(q);
struct bio_vec *bv = bio->bi_io_vec + bio->bi_vcnt;
struct page **pages = (struct page **)bv;
@@ -1469,10 +1469,10 @@ void bio_endio(struct bio *bio)
return;

if (bio->bi_bdev && bio_flagged(bio, BIO_TRACKED))
- rq_qos_done_bio(bio->bi_bdev->bd_disk->queue, bio);
+ rq_qos_done_bio(bdev_get_queue(bio->bi_bdev), bio);

if (bio->bi_bdev && bio_flagged(bio, BIO_TRACE_COMPLETION)) {
- trace_block_bio_complete(bio->bi_bdev->bd_disk->queue, bio);
+ trace_block_bio_complete(bdev_get_queue(bio->bi_bdev), bio);
bio_clear_flag(bio, BIO_TRACE_COMPLETION);
}

--
2.33.0

2021-10-14 14:34:08

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 2/5] block: use bdev_get_queue() in bdev.c

Convert bdev->bd_disk->queue to bdev_get_queue(), it's uses a cached
queue pointer and so is fater.

Signed-off-by: Pavel Begunkov <[email protected]>
---
block/bdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 30ae5b5d5f91..384e5bf991d8 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -327,12 +327,12 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
if (!ops->rw_page || bdev_get_integrity(bdev))
return result;

- result = blk_queue_enter(bdev->bd_disk->queue, 0);
+ result = blk_queue_enter(bdev_get_queue(bdev), 0);
if (result)
return result;
result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
REQ_OP_READ);
- blk_queue_exit(bdev->bd_disk->queue);
+ blk_queue_exit(bdev_get_queue(bdev));
return result;
}

@@ -363,7 +363,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,

if (!ops->rw_page || bdev_get_integrity(bdev))
return -EOPNOTSUPP;
- result = blk_queue_enter(bdev->bd_disk->queue, 0);
+ result = blk_queue_enter(bdev_get_queue(bdev), 0);
if (result)
return result;

@@ -376,7 +376,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
clean_page_buffers(page);
unlock_page(page);
}
- blk_queue_exit(bdev->bd_disk->queue);
+ blk_queue_exit(bdev_get_queue(bdev));
return result;
}

--
2.33.0

2021-10-14 15:43:19

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 5/5] block: convert the rest of block to bdev_get_queue

Convert bdev->bd_disk->queue to bdev_get_queue(), it's uses a cached
queue pointer and so is fater.

Signed-off-by: Pavel Begunkov <[email protected]>
---
block/bio-integrity.c | 2 +-
block/blk-cgroup.c | 16 ++++++++--------
block/blk-crypto.c | 2 +-
block/blk-iocost.c | 12 ++++++------
block/blk-merge.c | 2 +-
block/blk-mq.c | 2 +-
block/blk-throttle.c | 2 +-
block/genhd.c | 4 ++--
block/partitions/core.c | 4 ++--
9 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 21234ff966d9..d25114715459 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -134,7 +134,7 @@ int bio_integrity_add_page(struct bio *bio, struct page *page,
iv = bip->bip_vec + bip->bip_vcnt;

if (bip->bip_vcnt &&
- bvec_gap_to_prev(bio->bi_bdev->bd_disk->queue,
+ bvec_gap_to_prev(bdev_get_queue(bio->bi_bdev),
&bip->bip_vec[bip->bip_vcnt - 1], offset))
return 0;

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index eb48090eefce..cec86a705c89 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -621,7 +621,7 @@ struct block_device *blkcg_conf_open_bdev(char **inputp)
*/
int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
char *input, struct blkg_conf_ctx *ctx)
- __acquires(rcu) __acquires(&bdev->bd_disk->queue->queue_lock)
+ __acquires(rcu) __acquires(&bdev->bd_queue->queue_lock)
{
struct block_device *bdev;
struct request_queue *q;
@@ -632,7 +632,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
if (IS_ERR(bdev))
return PTR_ERR(bdev);

- q = bdev->bd_disk->queue;
+ q = bdev_get_queue(bdev);

rcu_read_lock();
spin_lock_irq(&q->queue_lock);
@@ -737,9 +737,9 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep);
* with blkg_conf_prep().
*/
void blkg_conf_finish(struct blkg_conf_ctx *ctx)
- __releases(&ctx->bdev->bd_disk->queue->queue_lock) __releases(rcu)
+ __releases(&ctx->bdev->bd_queue->queue_lock) __releases(rcu)
{
- spin_unlock_irq(&ctx->bdev->bd_disk->queue->queue_lock);
+ spin_unlock_irq(&bdev_get_queue(ctx->bdev)->queue_lock);
rcu_read_unlock();
blkdev_put_no_open(ctx->bdev);
}
@@ -842,7 +842,7 @@ static void blkcg_fill_root_iostats(void)
while ((dev = class_dev_iter_next(&iter))) {
struct block_device *bdev = dev_to_bdev(dev);
struct blkcg_gq *blkg =
- blk_queue_root_blkg(bdev->bd_disk->queue);
+ blk_queue_root_blkg(bdev_get_queue(bdev));
struct blkg_iostat tmp;
int cpu;

@@ -1801,7 +1801,7 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,

rcu_read_lock();
blkg = blkg_lookup_create(css_to_blkcg(css),
- bio->bi_bdev->bd_disk->queue);
+ bdev_get_queue(bio->bi_bdev));
while (blkg) {
if (blkg_tryget(blkg)) {
ret_blkg = blkg;
@@ -1837,8 +1837,8 @@ void bio_associate_blkg_from_css(struct bio *bio,
if (css && css->parent) {
bio->bi_blkg = blkg_tryget_closest(bio, css);
} else {
- blkg_get(bio->bi_bdev->bd_disk->queue->root_blkg);
- bio->bi_blkg = bio->bi_bdev->bd_disk->queue->root_blkg;
+ blkg_get(bdev_get_queue(bio->bi_bdev)->root_blkg);
+ bio->bi_blkg = bdev_get_queue(bio->bi_bdev)->root_blkg;
}
}
EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css);
diff --git a/block/blk-crypto.c b/block/blk-crypto.c
index 103c2e2d50d6..8f53f4a1f9e2 100644
--- a/block/blk-crypto.c
+++ b/block/blk-crypto.c
@@ -280,7 +280,7 @@ bool __blk_crypto_bio_prep(struct bio **bio_ptr)
* Success if device supports the encryption context, or if we succeeded
* in falling back to the crypto API.
*/
- if (blk_ksm_crypto_cfg_supported(bio->bi_bdev->bd_disk->queue->ksm,
+ if (blk_ksm_crypto_cfg_supported(bdev_get_queue(bio->bi_bdev)->ksm,
&bc_key->crypto_cfg))
return true;

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index b3880e4ba22a..a5b37cc65b17 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3165,12 +3165,12 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
if (IS_ERR(bdev))
return PTR_ERR(bdev);

- ioc = q_to_ioc(bdev->bd_disk->queue);
+ ioc = q_to_ioc(bdev_get_queue(bdev));
if (!ioc) {
- ret = blk_iocost_init(bdev->bd_disk->queue);
+ ret = blk_iocost_init(bdev_get_queue(bdev));
if (ret)
goto err;
- ioc = q_to_ioc(bdev->bd_disk->queue);
+ ioc = q_to_ioc(bdev_get_queue(bdev));
}

spin_lock_irq(&ioc->lock);
@@ -3332,12 +3332,12 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
if (IS_ERR(bdev))
return PTR_ERR(bdev);

- ioc = q_to_ioc(bdev->bd_disk->queue);
+ ioc = q_to_ioc(bdev_get_queue(bdev));
if (!ioc) {
- ret = blk_iocost_init(bdev->bd_disk->queue);
+ ret = blk_iocost_init(bdev_get_queue(bdev));
if (ret)
goto err;
- ioc = q_to_ioc(bdev->bd_disk->queue);
+ ioc = q_to_ioc(bdev_get_queue(bdev));
}

spin_lock_irq(&ioc->lock);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 762da71f9fde..c96f29f398fc 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -305,7 +305,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
*/
void __blk_queue_split(struct bio **bio, unsigned int *nr_segs)
{
- struct request_queue *q = (*bio)->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue((*bio)->bi_bdev);
struct bio *split = NULL;

switch (bio_op(*bio)) {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f42cf615c527..5cb5dd81a1d5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2223,7 +2223,7 @@ static inline unsigned short blk_plug_max_rq_count(struct blk_plug *plug)
*/
void blk_mq_submit_bio(struct bio *bio)
{
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
const int is_sync = op_is_sync(bio->bi_opf);
const int is_flush_fua = op_is_flush(bio->bi_opf);
struct request *rq;
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 8cefd14deed5..39bb6e68a9a2 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2063,7 +2063,7 @@ void blk_throtl_charge_bio_split(struct bio *bio)

bool __blk_throtl_bio(struct bio *bio)
{
- struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bio->bi_bdev);
struct blkcg_gq *blkg = bio->bi_blkg;
struct throtl_qnode *qn = NULL;
struct throtl_grp *tg = blkg_to_tg(blkg);
diff --git a/block/genhd.c b/block/genhd.c
index e11ee23a4401..901bef22f186 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -861,7 +861,7 @@ ssize_t part_stat_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct block_device *bdev = dev_to_bdev(dev);
- struct request_queue *q = bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bdev);
struct disk_stats stat;
unsigned int inflight;

@@ -905,7 +905,7 @@ ssize_t part_inflight_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct block_device *bdev = dev_to_bdev(dev);
- struct request_queue *q = bdev->bd_disk->queue;
+ struct request_queue *q = bdev_get_queue(bdev);
unsigned int inflight[2];

if (queue_is_mq(q))
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 3a4898433c43..9dbddc355b40 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -204,7 +204,7 @@ static ssize_t part_alignment_offset_show(struct device *dev,
struct block_device *bdev = dev_to_bdev(dev);

return sprintf(buf, "%u\n",
- queue_limit_alignment_offset(&bdev->bd_disk->queue->limits,
+ queue_limit_alignment_offset(&bdev_get_queue(bdev)->limits,
bdev->bd_start_sect));
}

@@ -214,7 +214,7 @@ static ssize_t part_discard_alignment_show(struct device *dev,
struct block_device *bdev = dev_to_bdev(dev);

return sprintf(buf, "%u\n",
- queue_limit_discard_alignment(&bdev->bd_disk->queue->limits,
+ queue_limit_discard_alignment(&bdev_get_queue(bdev)->limits,
bdev->bd_start_sect));
}

--
2.33.0

2021-10-18 03:44:57

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/5] cache request_queue pointer

On 10/14/21 8:03 AM, Pavel Begunkov wrote:
> Cache request_queue in bdev and replace two derefs in
> bdev->bd_disk->queue with bdev->bd_queue. Benchmarking
> with nullblk gave me around +1% to peak perf.
>
> All patches are self contained and don't rely on others from
> the set including 1/5 and can be taken separately. And some
> changes go in separate patches to minimise conflicts. When
> we agree on the approach, I'll send the rest converting some
> other spots out of block.

Looks fine to me. Christoph, any concerns?

One note, though - s/fater/faster in patches 2..5 in the commit
message.

--
Jens Axboe

2021-10-18 03:49:19

by Pavel Begunkov

[permalink] [raw]
Subject: Re: [PATCH 0/5] cache request_queue pointer

On 10/17/21 12:59, Jens Axboe wrote:
> On 10/14/21 8:03 AM, Pavel Begunkov wrote:
>> Cache request_queue in bdev and replace two derefs in
>> bdev->bd_disk->queue with bdev->bd_queue. Benchmarking
>> with nullblk gave me around +1% to peak perf.
>>
>> All patches are self contained and don't rely on others from
>> the set including 1/5 and can be taken separately. And some
>> changes go in separate patches to minimise conflicts. When
>> we agree on the approach, I'll send the rest converting some
>> other spots out of block.
>
> Looks fine to me. Christoph, any concerns?
>
> One note, though - s/fater/faster in patches 2..5 in the commit
> message.

Noted. I expect there will be a bunch of conflicts, I'll resend
it, hopefully once you refined and posted some of your stuff.

--
Pavel Begunkov

2021-10-18 08:26:09

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 0/5] cache request_queue pointer

On Sun, Oct 17, 2021 at 06:59:22AM -0600, Jens Axboe wrote:
> > All patches are self contained and don't rely on others from
> > the set including 1/5 and can be taken separately. And some
> > changes go in separate patches to minimise conflicts. When
> > we agree on the approach, I'll send the rest converting some
> > other spots out of block.
>
> Looks fine to me. Christoph, any concerns?

No huge fan of the extra pointer, but if it helps..