Bug fix: Complete the page writeback immediately if zfs_putpage was called with WB_SYNC_NONE #12776
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alled with WB_SYNC_NONE
gmelikov
reviewed
Nov 19, 2021
| @@ -3592,7 +3592,9 @@ zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc) | |||
| err = sa_bulk_update(zp->z_sa_hdl, bulk, cnt, tx); | |||
|
|
|||
| zfs_log_write(zfsvfs->z_log, tx, TX_WRITE, zp, pgoff, pglen, 0, | |||
| zfs_putpage_commit_cb, pp); | |||
| wbc->sync_mode != WB_SYNC_NONE ? zfs_putpage_commit_cb : NULL, | |||
| wbc->sync_mode != WB_SYNC_NONE ? pp : NULL); | |||
There was a problem hiding this comment.
Two same checks - maybe just use one usual if?
|
I think that the current fix is not good because the other thread doing the msync (with MS_SYNC) will complete immediately instead of waiting for the write to complete (which puts data integrity at risk in case of power loss). Secondly, it also breaks the kernel API semantics - PG_writeback should be set until the write is on stable storage. I'll close this PR and work on an alternate fix. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Fixes: #12662
Description
When the kernel occasionally performs a background page writeback,
WB_SYNC_NONEis specified as sync mode. Currently it appears that the page stays in the page writeback state until the transaction group is synced which then triggerszfs_putpage_commit_cband unsets the page writeback bit. This blocks other threads that are doing an msync with MS_SYNC for several seconds until the txg is synced due to the txg timeout.Given that WB_SYNC_NONE is specified, if I got it correctly, the correct behaviour should be that there is no need for the kernel to wait for the write to be done to stable storage, so in the fix I've completed the callback immediately if WB_SYNC_NONE is specified.
I'm new to the zfs/kernel source and just wanted some feedback to validate if the problem description/solution makes sense before making more progress on it (I still need to test the fix)
How Has This Been Tested?
PR still in draft to validate if the approach is correct.
Types of changes
Checklist:
Signed-off-by.