[PATCHSET,RFC,v2,0/5] Cache issue side time querying

Message

Jens Axboe Jan. 16, 2024, 4:54 p.m. UTC

Hi,

When I run my peak testing to see if we've regressed, my test script
always does:

	echo 0 > /sys/block/$DEV/queue/iostats
	echo 2 > /sys/block/$DEV/queue/nomerges

for each device being used. It's unfortunate that we need to disable
iostats, but without doing that, I lose about 12% performance. The main
reason for that is the time querying we need to do, when iostats are
enabled. As it turns out, lots of other block code is quite trigger
happy with querying time as well. We do have some nice batching in place
which helps ammortize that, but it's not perfect.

This trivial patchset simply caches the current time in struct blk_plug,
on the premise that any issue side time querying can get adequate
granularity through that. Nobody really needs nsec granularity on the
timestamp.

Results in patch 2, but tldr is a more than 9% improvement (108M -> 118M
IOPS) for my test case, which doesn't even enable most of the costly
block layer items that you'd typically find in a distro and which would
further increase the number of issue side time calls. This brings iostats
enabled _almost_ to the level of turning it off.

v2:
	- Fix typo in cover letter, the prep script obviously turns
	  _off_ iostats normally
	- Cover rest of block/* cases that use ktime_get_ns()
	- Fix build error in block/blk-wbt.c
	- Don't use the LSB to detect if the timestamp is valid or not,
	  just accept we'll do double ktime_get_ns() if we happen to
	  get 0 as a valid time.
	- Invalidate timestamp on any schedule out condition
	- Add two patches reclaiming the added space in blk_plug
	- Update to current perf results

Comments

Jens Axboe Jan. 16, 2024, 9:08 p.m. UTC | #1

On 1/16/24 9:54 AM, Jens Axboe wrote:
> Results in patch 2, but tldr is a more than 9% improvement (108M -> 118M
> IOPS) for my test case, which doesn't even enable most of the costly
> block layer items that you'd typically find in a distro and which would
> further increase the number of issue side time calls. This brings iostats
> enabled _almost_ to the level of turning it off.

Enabled the typical distro things (block cgroups, blk-wbt, iocost,
iolatency) which all add considerable cost (and is an optimization
project in itself) and this is the performance of the stock kernel with
iostats enabled:

IOPS=91.01M, BW=44.44GiB/s, IOS/call=32/32
IOPS=91.29M, BW=44.58GiB/s, IOS/call=31/32
IOPS=91.27M, BW=44.57GiB/s, IOS/call=32/31
IOPS=91.26M, BW=44.56GiB/s, IOS/call=32/31
IOPS=91.38M, BW=44.62GiB/s, IOS/call=32/31
IOPS=91.28M, BW=44.57GiB/s, IOS/call=32/32

which is down from 122M for an optimized config and with iostats off.
With this patchset applied (and one extra patch, missed a spot...), we
now get:

IOPS=101.38M, BW=49.50GiB/s, IOS/call=32/32
IOPS=101.31M, BW=49.47GiB/s, IOS/call=32/32
IOPS=101.35M, BW=49.49GiB/s, IOS/call=31/31
IOPS=101.44M, BW=49.53GiB/s, IOS/call=32/31
IOPS=101.32M, BW=49.47GiB/s, IOS/call=32/32
IOPS=101.14M, BW=49.38GiB/s, IOS/call=32/31

which is about a 10% improvement. Mostly ran this because I was curious,
and while the above config changes do add more time stamping, it also
adds additional overhead. In any case, 10% win for the distro config
case is not bad at all.