On 11/23/23 1:19 PM, Jens Axboe wrote:
> On 11/23/23 11:12 AM, Kanchan Joshi wrote:
>> On 11/23/2023 9:00 PM, Christoph Hellwig wrote:
>>> The rest looks good, but that stats overhead seems pretty horrible..
>>
>> On my setup
>> Before[1]: 7.06M
>> After[2]: 8.29M
>>
>> [1]
>> # taskset -c 2,3 t/io_uring -b512 -d256 -c32 -s32 -p1 -F1 -B1 -O0 -n2
>> -u1 -r4 /dev/ng0n1 /dev/ng1n1
>> submitter=0, tid=2076, file=/dev/ng0n1, node=-1
>> submitter=1, tid=2077, file=/dev/ng1n1, node=-1
>> polled=1, fixedbufs=1/0, register_files=1, buffered=1, QD=256
>> Engine=io_uring, sq_ring=256, cq_ring=256
>> polled=1, fixedbufs=1/0, register_files=1, buffered=1, QD=256
>> Engine=io_uring, sq_ring=256, cq_ring=256
>> IOPS=6.95M, BW=3.39GiB/s, IOS/call=32/31
>> IOPS=7.06M, BW=3.45GiB/s, IOS/call=32/32
>> IOPS=7.06M, BW=3.45GiB/s, IOS/call=32/31
>> Exiting on timeout
>> Maximum IOPS=7.06M
>>
>> [2]
>> # taskset -c 2,3 t/io_uring -b512 -d256 -c32 -s32 -p1 -F1 -B1 -O0 -n2
>> -u1 -r4 /dev/ng0n1 /dev/ng1n1
>> submitter=0, tid=2123, file=/dev/ng0n1, node=-1
>> submitter=1, tid=2124, file=/dev/ng1n1, node=-1
>> polled=1, fixedbufs=1/0, register_files=1, buffered=1, QD=256
>> Engine=io_uring, sq_ring=256, cq_ring=256
>> IOPS=8.27M, BW=4.04GiB/s, IOS/call=32/31
>> IOPS=8.29M, BW=4.05GiB/s, IOS/call=32/31
>> IOPS=8.29M, BW=4.05GiB/s, IOS/call=31/31
>> Exiting on timeout
>> Maximum IOPS=8.29M
>
> It's all really down to how expensive getting the current time is on
> your box, some will be better and some worse
>
> One idea that has been bounced around in the past is to have a
> blk_ktime_get_ns() and have it be something ala:
>
> u64 blk_ktime_get_ns(void)
> {
> struct blk_plug *plug = current->plug;
>
> if (!plug)
> return ktime_get_ns();
>
> if (!plug->ktime_valid)
> plug->ktime = ktime_get_ns();
>
> return plug->ktime;
> }
>
> in freestyle form, with the idea being that we don't care granularity to
> the extent that we'd need a new stamp every time.
>
> If the task is scheduled out, the plug is flushed anyway, which should
> invalidate the stamp. For preemption this isn't true iirc, so we'd need
> some kind of blk_flush_plug_ts() or something for that case to
> invalidate it.
>
> Hopefully this could then also get away from passing in a cached value
> that we do in various spots, exactly because all of this time stamping
> is expensive. It's also a bit of a game of whack-a-mole, as users get
> added and distro kernels tend to turn on basically everything anyway.
Did a quick'n dirty (see below), which recoups half of the lost
performance for me. And my kernel deliberately doesn't enable all of the
gunk in block/ that slows everything down, I suspect it'll be a bigger
win for you percentage wise:
IOPS=121.42M, BW=59.29GiB/s, IOS/call=31/31
IOPS=121.47M, BW=59.31GiB/s, IOS/call=32/32
IOPS=121.44M, BW=59.30GiB/s, IOS/call=31/31
IOPS=121.47M, BW=59.31GiB/s, IOS/call=31/31
IOPS=121.45M, BW=59.30GiB/s, IOS/call=32/32
IOPS=119.95M, BW=58.57GiB/s, IOS/call=31/32
IOPS=115.30M, BW=56.30GiB/s, IOS/call=32/31
IOPS=115.38M, BW=56.34GiB/s, IOS/call=32/32
IOPS=115.35M, BW=56.32GiB/s, IOS/call=32/32
diff --git a/block/blk-core.c b/block/blk-core.c
index fdf25b8d6e78..6e74af442b94 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1055,6 +1055,7 @@ void blk_start_plug_nr_ios(struct blk_plug *plug, unsigned short nr_ios)
plug->rq_count = 0;
plug->multiple_queues = false;
plug->has_elevator = false;
+ plug->cur_ktime = 0;
INIT_LIST_HEAD(&plug->cb_list);
/*
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 3f4d41952ef2..e4e9f7eed346 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -143,7 +143,7 @@ static void blk_account_io_flush(struct request *rq)
part_stat_lock();
part_stat_inc(part, ios[STAT_FLUSH]);
part_stat_add(part, nsecs[STAT_FLUSH],
- ktime_get_ns() - rq->start_time_ns);
+ blk_ktime_get_ns() - rq->start_time_ns);
part_stat_unlock();
}
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 089fcb9cfce3..d06cea625462 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -829,7 +829,7 @@ static int ioc_autop_idx(struct ioc *ioc, struct gendisk *disk)
/* step up/down based on the vrate */
vrate_pct = div64_u64(ioc->vtime_base_rate * 100, VTIME_PER_USEC);
- now_ns = ktime_get_ns();
+ now_ns = blk_ktime_get_ns();
if (p->too_fast_vrate_pct && p->too_fast_vrate_pct <= vrate_pct) {
if (!ioc->autop_too_fast_at)
@@ -1044,7 +1044,7 @@ static void ioc_now(struct ioc *ioc, struct ioc_now *now)
unsigned seq;
u64 vrate;
- now->now_ns = ktime_get();
+ now->now_ns = blk_ktime_get();
now->now = ktime_to_us(now->now_ns);
vrate = atomic64_read(&ioc->vtime_rate);
@@ -2810,7 +2810,7 @@ static void ioc_rqos_done(struct rq_qos *rqos, struct request *rq)
return;
}
- on_q_ns = ktime_get_ns() - rq->alloc_time_ns;
+ on_q_ns = blk_ktime_get_ns() - rq->alloc_time_ns;
rq_wait_ns = rq->start_time_ns - rq->alloc_time_ns;
size_nsec = div64_u64(calc_size_vtime_cost(rq, ioc), VTIME_PER_NSEC);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 900c1be1fee1..9c96dee9e584 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -323,7 +323,7 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
RB_CLEAR_NODE(&rq->rb_node);
rq->tag = BLK_MQ_NO_TAG;
rq->internal_tag = BLK_MQ_NO_TAG;
- rq->start_time_ns = ktime_get_ns();
+ rq->start_time_ns = blk_ktime_get_ns();
rq->part = NULL;
blk_crypto_rq_set_defaults(rq);
}
@@ -333,7 +333,7 @@ EXPORT_SYMBOL(blk_rq_init);
static inline void blk_mq_rq_time_init(struct request *rq, u64 alloc_time_ns)
{
if (blk_mq_need_time_stamp(rq))
- rq->start_time_ns = ktime_get_ns();
+ rq->start_time_ns = blk_ktime_get_ns();
else
rq->start_time_ns = 0;
@@ -444,7 +444,7 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
/* alloc_time includes depth and tag waits */
if (blk_queue_rq_alloc_time(q))
- alloc_time_ns = ktime_get_ns();
+ alloc_time_ns = blk_ktime_get_ns();
if (data->cmd_flags & REQ_NOWAIT)
data->flags |= BLK_MQ_REQ_NOWAIT;
@@ -629,7 +629,7 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
/* alloc_time includes depth and tag waits */
if (blk_queue_rq_alloc_time(q))
- alloc_time_ns = ktime_get_ns();
+ alloc_time_ns = blk_ktime_get_ns();
/*
* If the tag allocator sleeps we could get an allocation for a
@@ -1037,7 +1037,7 @@ static inline void __blk_mq_end_request_acct(struct request *rq, u64 now)
inline void __blk_mq_end_request(struct request *rq, blk_status_t error)
{
if (blk_mq_need_time_stamp(rq))
- __blk_mq_end_request_acct(rq, ktime_get_ns());
+ __blk_mq_end_request_acct(rq, blk_ktime_get_ns());
blk_mq_finish_request(rq);
@@ -1080,7 +1080,7 @@ void blk_mq_end_request_batch(struct io_comp_batch *iob)
u64 now = 0;
if (iob->need_ts)
- now = ktime_get_ns();
+ now = blk_ktime_get_ns();
while ((rq = rq_list_pop(&iob->req_list)) != NULL) {
prefetch(rq->bio);
@@ -1249,7 +1249,7 @@ void blk_mq_start_request(struct request *rq)
trace_block_rq_issue(rq);
if (test_bit(QUEUE_FLAG_STATS, &q->queue_flags)) {
- rq->io_start_time_ns = ktime_get_ns();
+ rq->io_start_time_ns = blk_ktime_get_ns();
rq->stats_sectors = blk_rq_sectors(rq);
rq->rq_flags |= RQF_STATS;
rq_qos_issue(q, rq);
@@ -3066,7 +3066,7 @@ blk_status_t blk_insert_cloned_request(struct request *rq)
blk_mq_run_dispatch_ops(q,
ret = blk_mq_request_issue_directly(rq, true));
if (ret)
- blk_account_io_done(rq, ktime_get_ns());
+ blk_account_io_done(rq, blk_ktime_get_ns());
return ret;
}
EXPORT_SYMBOL_GPL(blk_insert_cloned_request);
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 13e4377a8b28..5919919ba1c3 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1813,7 +1813,7 @@ static bool throtl_tg_is_idle(struct throtl_grp *tg)
time = min_t(unsigned long, MAX_IDLE_TIME, 4 * tg->idletime_threshold);
ret = tg->latency_target == DFL_LATENCY_TARGET ||
tg->idletime_threshold == DFL_IDLE_THRESHOLD ||
- (ktime_get_ns() >> 10) - tg->last_finish_time > time ||
+ (blk_ktime_get_ns() >> 10) - tg->last_finish_time > time ||
tg->avg_idletime > tg->idletime_threshold ||
(tg->latency_target && tg->bio_cnt &&
tg->bad_bio_cnt * 5 < tg->bio_cnt);
@@ -2058,7 +2058,7 @@ static void blk_throtl_update_idletime(struct throtl_grp *tg)
if (last_finish_time == 0)
return;
- now = ktime_get_ns() >> 10;
+ now = blk_ktime_get_ns() >> 10;
if (now <= last_finish_time ||
last_finish_time == tg->checked_last_finish_time)
return;
@@ -2325,7 +2325,7 @@ void blk_throtl_bio_endio(struct bio *bio)
if (!tg->td->limit_valid[LIMIT_LOW])
return;
- finish_time_ns = ktime_get_ns();
+ finish_time_ns = blk_ktime_get_ns();
tg->last_finish_time = finish_time_ns >> 10;
start_time = bio_issue_time(&bio->bi_issue) >> 10;
diff --git a/block/blk.h b/block/blk.h
index 08a358bc0919..4f081a00e644 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -518,4 +518,17 @@ static inline int req_ref_read(struct request *req)
return atomic_read(&req->ref);
}
+static inline u64 blk_ktime_get_ns(void)
+{
+ struct blk_plug *plug = current->plug;
+
+ if (!plug)
+ return ktime_get_ns();
+ if (!(plug->cur_ktime & 1ULL)) {
+ plug->cur_ktime = ktime_get_ns();
+ plug->cur_ktime |= 1ULL;
+ }
+ return plug->cur_ktime;
+}
+
#endif /* BLK_INTERNAL_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 51fa7ffdee83..081830279f70 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -972,6 +972,9 @@ struct blk_plug {
bool multiple_queues;
bool has_elevator;
+ /* bit0 set if valid */
+ u64 cur_ktime;
+
struct list_head cb_list; /* md requires an unplug callback */
};
--
Jens Axboe
