DAOS-19427 dfuse: defer cache invalidations off the worker pool - #18802
DAOS-19427 dfuse: defer cache invalidations off the worker pool#18802mchaarawi wants to merge 1 commit into
Conversation
|
Ticket title is 'NLT stage timing out (~63 min) on master and PR builds — daos container create fault injection appears to hang' |
15a1464 to
74216bd
Compare
dfuse worker threads are the sole readers of /dev/fuse and run a fixed-size pool. Request handlers called fuse_lowlevel_notify_inval_entry() inline; that blocks acquiring the parent's kernel i_rwsem, which a client may hold while waiting on the same worker pool, deadlocking the mount (e.g. test_alloc_fail_cont_create with concurrent UNS creates). Add an on-demand dentry invalidation queue drained by the existing invalidation thread and route the setxattr, lookup, rename, open and opendir notify_inval_entry calls through it. open/opendir transfer their held inode reference into the queue item so it is dropped after the notify. No /dev/fuse reader issues a blocking notify_inval_entry, breaking the cycle while preserving the invalidations. inval_inode/notify_delete (ioctl, unlink) are left synchronous: under writeback caching they block on dirty-page writeback that only a worker can service, so deferring them to the invalidation thread would hang the shutdown join once the workers have exited. The invalidation thread also stops issuing notifies once shutdown has begun so the join stays bounded. Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>
74216bd to
88b2340
Compare
mchaarawi
changed the title
|
@mchaarawi I started goofing around with making a fix, was going to tag you after it had passed (ran into unrelated NLT log size issues, attempting to fix by moving the new tests into their own "stress" section): #18806 Mine is either overengineered or potentially more complete, depending on how you look at it... :) Feel free to mine it for ideas. The new NLT stress tests might be useful for validating your approach. |
it is neither.. you are just simply doing too many things in one PR IMO.
|
actually i take that back.. it does look like you have more changes than i was reviewing. |
ok i should have reviewed your PR more closely, i appologize.. i posted some comments there. ultimately, i think my PR is simpler bec i just tried to resolve your finding that you posted in the ticket with fuse_lowlevel_notify_inval_entry(). but your PR goes beyond that. im not sure yet if it is all required or not yet. does it make sense to land this one first (if there are no functional issues) |
I'm fine with that. I'm running a local test with your fix and the new NLT stress tests to see how it does. Will post results when it's finished. Does your NLT work include something like #18779 ? If not, I'd like to get that into master because it was really helpful in finding this issue. |
it is here #18808 |
|
@mchaarawi: My local testing of your patch with the NLT reproducer seems to show that your patch fixes it. The repro fails pretty consistently on unpatched master. |
There was a problem hiding this comment.
LGTM, and passes the stress tests I added in my PR.
2 participants
dfuse worker threads are the sole readers of /dev/fuse and run a fixed-size pool. Request handlers called fuse_lowlevel_notify_inval_entry() and friends inline; those block on kernel inode locks (parent i_rwsem) that a client may hold while waiting on the same worker pool, deadlocking the mount (e.g. test_alloc_fail_cont_create with concurrent UNS creates).
Add an on-demand invalidation queue drained by the existing invalidation thread and route the setxattr, lookup, open, opendir, and rename notify calls through it. open/opendir transfer their held inode reference into the queue item so it is dropped after the notify. No /dev/fuse reader issues a blocking notify, breaking the cycle while preserving the invalidations.
Steps for the author:
After all prior steps are complete: