Re: [PATCH v3 1/1] virtio-blk: avoid preallocating big SGL for data
From: Jens Axboe <axboe@kernel.dk>
Date: 2021-09-02 02:08:28
Also in:
kvm, linux-block
On 9/1/21 4:25 PM, Max Gurtovoy wrote:
On 9/1/2021 6:27 PM, Jens Axboe wrote:quoted
On 9/1/21 8:58 AM, Max Gurtovoy wrote:quoted
On 9/1/2021 5:50 PM, Michael S. Tsirkin wrote:quoted
On Wed, Sep 01, 2021 at 04:14:34PM +0300, Max Gurtovoy wrote:quoted
No need to pre-allocate a big buffer for the IO SGL anymore. If a device has lots of deep queues, preallocation for the sg list can consume substantial amounts of memory. For HW virtio-blk device, nr_hw_queues can be 64 or 128 and each queue's depth might be 128. This means the resulting preallocation for the data SGLs is big. Switch to runtime allocation for SGL for lists longer than 2 entries. This is the approach used by NVMe drivers so it should be reasonable for virtio block as well. Runtime SGL allocation has always been the case for the legacy I/O path so this is nothing new. The preallocated small SGL depends on SG_CHAIN so if the ARCH doesn't support SG_CHAIN, use only runtime allocation for the SGL. Re-organize the setup of the IO request to fit the new sg chain mechanism. No performance degradation was seen (fio libaio engine with 16 jobs and 128 iodepth): IO size IOPs Rand Read (before/after) IOPs Rand Write (before/after) -------- --------------------------------- ---------------------------------- 512B 318K/316K 329K/325K 4KB 323K/321K 353K/349K 16KB 199K/208K 250K/275K 128KB 36K/36.1K 39.2K/41.7K Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com> Reviewed-by: Israel Rukshin <redacted>Could you use something to give confidence intervals maybe? As it is it looks like a 1-2% regression for 512B and 4KB.1%-2% is not a regression. It's a device/env/test variance. This is just one test results. I run it many times and got difference by +/- 2%-3% in each run for each sides. Even if I run same driver without changes I get 2%-3% difference between runs. If you have a perf test suite for virtio-blk it will be great if you can run it, or maybe Feng Li has.You're adding an allocation to the hot path, and a free to the completion hot path. It's not unreasonable to expect that there could be performance implications associated with that. Which would be particularly evident with 1 segment requests, as the results would seem to indicate as well.but for sg_nents <= 2 there is no dynamic allocation also in this patch exactly as we do in nvmf RDMA and FC for example.
My quick read missed that, which is why you're using chaining. Then it looks very reasonable to me.
quoted
Probably needs better testing. A profile of a peak run before and after and a diff of the two might also be interesting.I'll run ezfio test suite with stronger virtio-blk device that reach > 800KIOPs
That'd be better, and preferably a test with pinning etc so that you can show more consistent results. Just reading your table does indeed look like there's a performance degradation, even if you preface it by saying there is none. It would need better explaining, but preferably better testing.
quoted
The common idiom for situations like this is to have an inline part that holds 1-2 segments, and then only punt to alloc if you need more than that. As the number of segments grows, the cost per request matters less.isn't this the case here ? or am I missing something ?
it totally is, I was the one missing that. -- Jens Axboe _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization