On Wed, Sep 28, 2022 at 06:07:34AM +0000, Lu, Davina wrote:
Hello,
My analyzing is that the degradation is introduce by commit {244adf6426ee31a83f397b700d964cff12a247d3} and the issue is the contention on rsv_conversion_wq. The simplest option is to increase the journal size, but that introduces more operational complexity. Another option is to add the following change in attachment "allow more ext4-rsv-conversion workqueue.patch"
Hi, thanks for the patch. However, I don't believe as written it is safe. That's because your patch will allow multiple CPU's to run ext4_flush_completed_IO(), and this function is not set up to be safe to be run concurrently. That is, I don't see the necessary locking if we have two CPU's trying to convert unwritten extents on the same inode.
It could be made safe, and certainly if the problem is that you are worried about contention across multiple inodes being written to by different FIO jobs, then certainly this could be a potential contention issue.
However, what doesn't make sense is that increasing the journal size also seems to fix the issue for you. That implies the problem is one of the journal being to small, and so you are running into an issue of stalls caused by the need to do a synchronous checkpoint to clear space in the journal. That is a different problem than one of there being a contention problem with rsv_conversion_wq.
So I want to make sure we understand what you are seeing before we make such a change. One potential concern is that will cause a large number of additional kernel threads. Now, if these extra kernel threads are doing useful work, then that's not necessarily an issue. But if not, then it's going to be burning a large amount of kernel memory (especially for a system with a large number of CPU cores).
Regards,
- Ted