Repository navigation
fuse: throttle foreground io-uring requests with the writeback cache - #244
Conversation
With the writeback cache, foreground requests can wait in userspace on DLM revokes. A revoke waits on page invalidation, which waits on writeback. If foreground requests held every entry of an io-uring queue, background writeback could not be sent and none of them would complete. When the connection has the writeback cache, each queue now keeps a share of its entries for background requests. Background requests may use a quarter of the entries, but at least 2. Foreground requests may use the rest, but at least 8. A foreground request over the limit waits on the new fuse_req_fg_queue and only moves to fuse_req_queue when a slot frees up. Background requests keep their own path to free entries and are held to their cap, except that each queue may always have one active background request. If a queue has fewer than 10 entries both minimums cannot hold, and the foreground minimum wins. The foreground count is released on completion, on commit errors and on entry teardown. Requests marked no_fg_limit skip the foreground limit. These are the page cache reads and writes (redfs_do_readfolio, redfs_send_readpages and redfs_send_write_pages) and the flush from write_inode. A page fault holds the inode range lock while its read is outstanding, and the revoke that is waiting for that range lock would never finish if the read were queued behind other foreground requests. The flush is sent by writeback itself. Without the writeback cache, dispatch is unchanged. Signed-off-by: Hai Zhong Zhou <hazhou@ddn.com>
|
@hbirth Here I'm back to create PR based on my "forked DDN Linux" repo, just in case I have more subsequent commits coming... Please take a review here instead. Thanks. |
|
As I said in the other PR. I don't think this is a real problem in the issues we're seeing, but I'm in no position to decline, so ... here we go. In the tests that showed the hang we never had many waiting fuse requests (highest number around 3) |
Horst, not sure which case you did review.... For the cases I investigated, my VM has two io-uring queues and each queue depth is 8, and total 16 entries were consumed by FUSE requests, then waiting on the invalidation completion. So this is a real problem, not "imagine" problem:-) |
611e47d
into
DDNStorage:redfs-ubuntu-hwe-6.17.0-16.16-24.04.1
| * invalidation, which waits on writeback - if they held all entries, | ||
| * background writeback could not be sent and none of them would complete. | ||
| */ | ||
| #define FUSE_URING_MIN_FOREGROUND 8U |
There was a problem hiding this comment.
I don't think this is used any where, so this can probably come out unless there was some use case that was forgotten
There was a problem hiding this comment.
Yes, Allison. Here I need to refine it for the calculation... but team is rushing to provide the testing, thus quickly merging the PR before I complete my another working commit... so I have to create another PR for my next commit for the io-uring change...
| return queue->nr_ents; | ||
| } | ||
|
|
||
| return queue->nr_ents - bg; |
There was a problem hiding this comment.
was FUSE_URING_MIN_FOREGROUND maybe supposed to go here? This works out to nr_ents - max(nr_ents/4, 2). I think (max(nr_ents - bg, min(FUSE_URING_MIN_FOREGROUND, nr_ents))) makes more sense?
That would match the commit message "Foreground requests may use the rest, but at least 8" and "if a queue has fewer than 10 entries ... the foreground minimum wins."
There was a problem hiding this comment.
You're right. Here I need to revise it too... Anyway, I will have another commit to contains this. Thanks.
| spin_lock(&queue->lock); | ||
| ent->cmd = cmd; | ||
| queue->nr_ents++; | ||
| fuse_uring_flush_queue_fg(queue); |
There was a problem hiding this comment.
nit: since the queue is still empty here, we're going to get a warning every time the module loads. I wonder if we need the flush at this point since we're still registering?
There was a problem hiding this comment.
good point! Let me think about this, and address in the working commit if needed. Thanks, and please help to continue review the codes, then I can try to address all these in one commit:-)
| /* remove entry from queue->fpq->processing */ | ||
| list_del_init(&req->list); | ||
| if (test_and_clear_bit(FR_URING_FG, &req->flags)) | ||
| queue->active_foreground--; |
There was a problem hiding this comment.
do we need a fuse_uring_flush_queue_fg for decrementing the foreground?
There was a problem hiding this comment.
not sure for now, will check it later.
|
nits aside this looks good. istr you mentioned a server side companion fix during the meeting? maybe link it here when you get a chance. Thanks! |
Yes, I do not yet create the userspace PR yet, which is used to set or configure the io-uring depth to have "reserved" slots other than the default 8 entries per queue. Will put it here once I have it. Thank you for your review comments! |
3 participants
FUSE IO-URING: add throttling mechanism for the fuse foreground worker threads to preserve the io-uring entries to the write-back background tasks, otherwise, no chance to complete the background tasks but the foreground worker threads are waiting on the background tasks, then form deadlock issues.
With the writeback cache, foreground requests can wait in userspace on DLM revokes. A revoke waits on page invalidation, which waits on writeback. If foreground requests held every entry of an io-uring queue, background writeback could not be sent and none of them would complete.
When the connection has the writeback cache, each queue now keeps a share of its entries for background requests. Background requests may use a quarter of the entries, but at least 2. Foreground requests may use the rest, but at least 8. A foreground request over the limit waits on the new fuse_req_fg_queue and only moves to fuse_req_queue when a slot frees up. Background requests keep their own path to free entries and are held to their cap, except that each queue may always have one active background request. If a queue has fewer than 10 entries both minimums cannot hold, and the foreground minimum wins. The foreground count is released on completion, on commit errors and on entry teardown.
Requests marked no_fg_limit skip the foreground limit. These are the page cache reads and writes (redfs_do_readfolio, redfs_send_readpages and redfs_send_write_pages) and the flush from write_inode. A page fault holds the inode range lock while its read is outstanding, and the revoke that is waiting for that range lock would never finish if the read were queued behind other foreground requests. The flush is sent by writeback itself. Without the writeback cache, dispatch is unchanged.