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
