cache request_queue pointer
Subsystems (2)
cover 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(-)
Signature · 1 line
2.33.0
PATCH 1/5 block: cache request queue in bdev
There are tons of places where we need to get a request_queue only having bdev, which turns into bdev->bd_disk->queue. There are probably a hundred of such places considering inline helpers, and enough of them are in hot paths.
Cache queue pointer in struct block_device and make use of it in bdev_get_queue().
Signed-off-by Pavel Begunkov <[email protected]>
--- block/bdev.c | 1 + block/genhd.c | 4 +++- include/linux/blk_types.h | 1 + include/linux/blkdev.h | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-)
block/bdev.c +1 -0
@@ -494,6 +494,7 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) bdev->bd_disk = disk; bdev->bd_partno = partno; bdev->bd_inode = inode; + bdev->bd_queue = disk->queue; bdev->bd_stats = alloc_percpu(struct disk_stats);if (!bdev->bd_stats) {iput (inode);
block/genhd.c +3 -1
@@ -1244,6 +1244,9 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,if (!disk->bdi) goto out_free_disk; +/* bdev_alloc() might need the queue, set before the first call */ + disk->queue = q; + disk->part0 =bdev_alloc (disk,0 );if (!disk->part0) goto out_free_bdi;
@@ -1259,7 +1262,6 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,disk_to_dev (disk)->type = &disk_type;device_initialize (disk_to_dev (disk));inc_diskseq (disk); - disk->queue = q; q->disk = disk;lockdep_init_map (&disk->lockdep_map,"(bio completion)" , lkclass,0 );#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
include/linux/blk_types.h +1 -0
@@ -38,6 +38,7 @@ struct block_device {u8 bd_partno;spinlock_t bd_size_lock;/* for bd_inode->i_size updates */ struct gendisk * bd_disk; +struct request_queue * bd_queue;/* The counter of freeze processes */ int bd_fsfreeze_count;
include/linux/blkdev.h +1 -1
@@ -574,7 +574,7 @@ int iocb_bio_iopoll(struct kiocb *kiocb, unsigned int flags);static inline struct request_queue *bdev_get_queue (struct block_device *bdev) { -return bdev->bd_disk->queue;/* this is never NULL */ +return bdev->bd_queue;/* this is never NULL */ }/*
Signature · 1 line
2.33.0
PATCH 2/5 block: use bdev_get_queue() in bdev.c
In reply to Pavel BegunkovConvert 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(-)
block/bdev.c +4 -4
@@ -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; }
Signature · 1 line
2.33.0
PATCH 3/5 block: use bdev_get_queue() in bio.c
In reply to Pavel BegunkovConvert 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(-)
block/bio.c +5 -5
@@ -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 ); }
Signature · 1 line
2.33.0
PATCH 4/5 block: use bdev_get_queue() in blk-core.c
In reply to Pavel BegunkovConvert 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(-)
block/blk-core.c +5 -5
@@ -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);
Signature · 1 line
2.33.0
PATCH 5/5 block: convert the rest of block to bdev_get_queue
In reply to Pavel BegunkovConvert 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(-)
block/bio-integrity.c +1 -1
@@ -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 ;
block/blk-cgroup.c +8 -8
@@ -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);
block/blk-crypto.c +1 -1
@@ -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;
block/blk-iocost.c +6 -6
@@ -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);
block/blk-merge.c +1 -1
@@ -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)) {
block/blk-mq.c +1 -1
@@ -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;
block/blk-throttle.c +1 -1
@@ -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);
block/genhd.c +2 -2
@@ -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))
block/partitions/core.c +2 -2
@@ -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)); }
Signature · 1 line
2.33.0
Jens Axboe
maintainerQuoted 9 lines · Pavel Begunkov
Cache
request_queuein bdev and replace two derefs inbdev->bd_disk->queuewithbdev->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.
Signature · 1 line
Jens Axboe
Quoted 11 lines · Jens Axboe
Quoted 9 lines · Pavel Begunkov
Cache
request_queuein bdev and replace two derefs inbdev->bd_disk->queuewithbdev->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.
Quoted 4 lines
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.
Signature · 1 line
Pavel Begunkov
Quoted 7 lines · Jens Axboe
Quoted 5 lines
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..