Re: [PATCH 06/26] block: Introduce zone write plugging

Damien Le Moal
2024-02-05T06:14:59Z

On 2/5/24 14:50, Christoph Hellwig wrote:
> On Mon, Feb 05, 2024 at 02:37:41PM +0900, Damien Le Moal wrote:
>> OK. So I think that Hannes'idea to get/put the queue usage counter reference
>> based on a zone BIO plug becoming not empty (get ref) and becoming empty (put
>> ref) may be simpler then. And that would also work in the same way for blk-mq
>> and BIO based drivers.
>
> Maybe I'm missing something, but I'm not sure how that would even work.
>
> We need a q_usage_counter ref when doing all the submissions checks
> (limits, bounce, etc) early in blk_mq_sunmit_bio, and that one should be
> taken using the mormal bio_queue_enter patch to do the right thing on
> nowait submissions, when the queue is already frozen, etc. What
> is the benefit of not just keeping that references vs releasing it
> for all but the first bio just so that we need to grab another
> new reference at the actual submission time?

I just tried to make the change, but the code does not become easier/cleaner at
all. In fact, the contrary, it is very messy. I think I am going to keep things
as is regarding the ref counting.

There is one thing I need to check though: I re-ran the perf tests but this time
took the average of 10 runs to mitigate differences due to variance between runs
of the same test. And doing that, I do see a regression in performance for ZNS
4K qd=16 sequential writes (751 MB/s with rc2 vs 661 MB/s with ZWP). I need to
check if that is due to never using the cached request for plugged BIOs in
blk_mq_submit_bio(). If that is the case, we'll need to tweak the reference
dropping there for the cached request case.

I am also seeing a regression in performance with btrfs on SMR HDD (244 MB/s
with rc2 and block/for-next vs 233 MB/s with ZWP). I need to check that as well.

--
Damien Le Moal
Western Digital Research