Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Linux 6.5] Use copy_splice_read instead of filemap_splice_read #15164

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

ckane
Copy link
Contributor

@ckane ckane commented Aug 8, 2023

Motivation and Context

A rare data corruption bug was encountered during the filemap_splice_read change.

Issue #15163 documents it, and this PR should fix that.

Description

Replace filemap_splice_read with copy_splice_read and update the autoconf tests to look for the latter instead of the former

How Has This Been Tested?

Tested the failure cases in the linked issue. Run-tested in a VM on 6.5rc4. Build tested.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Signed-off-by: Coleman Kane <[email protected]>
@ckane
Copy link
Contributor Author

ckane commented Aug 8, 2023

@satmandu @behlendorf @rincebrain - hopefully fixes #15163 and last blocking item for 6.5 compatibility

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 8, 2023
@satmandu
Copy link
Contributor

satmandu commented Aug 8, 2023

@satmandu @behlendorf @rincebrain - hopefully fixes #15163 and last blocking item for 6.5 compatibility

I don't think this would also resolve #15145 for 6.5 compatibility though...

@ckane
Copy link
Contributor Author

ckane commented Aug 8, 2023

@satmandu @behlendorf @rincebrain - hopefully fixes #15163 and last blocking item for 6.5 compatibility

I don't think this would also resolve #15145 for 6.5 compatibility though...

Yeah, I recognize that this doesn't fix the additional issue when --enable-ubsan is added, which should still be fixed too, but for the default configure options, this should hopefully make it usable.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 8, 2023
@behlendorf behlendorf merged commit 8ce2eba into openzfs:master Aug 8, 2023
18 of 19 checks passed
stgraber pushed a commit to zabbly/zfs that referenced this pull request Sep 5, 2023
Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Coleman Kane <[email protected]>
Closes openzfs#15164
stgraber pushed a commit to zabbly/zfs that referenced this pull request Sep 8, 2023
Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Coleman Kane <[email protected]>
Closes openzfs#15164
behlendorf pushed a commit that referenced this pull request Sep 19, 2023
Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Coleman Kane <[email protected]>
Closes #15164
tonyhutter pushed a commit that referenced this pull request Sep 27, 2023
Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Coleman Kane <[email protected]>
Closes #15164
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
Using the filemap_splice_read function for the splice_read handler was
leading to occasional data corruption under certain circumstances. Favor
using copy_splice_read instead, which does not demonstrate the same
erroneous behavior under the tested failure cases.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Coleman Kane <[email protected]>
Closes openzfs#15164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants