Temporarily disable SEEK_HOLE to workaround #11900 #12710
Conversation
|
Could you please describe the problem that this addresses in this PR? I clicked through to #11900 but could not find a succinct description of the problem. In what circumstance does SEEK_HOLE return the wrong value? What code/design problem causes that? |
|
If I had a more comprehensive report of what goes wrong, I'd be iterating on @behlendorf's attempts at fixing it, but a summary of what prompted this is "coreutils 9 shipped with SEEK_DATA/SEEK_HOLE use enabled by default, and if you cp with this soon enough after writing something[1] on ZFS, this results in files with correct sizes but gaps where none exist on the source". And since "cp produces wrong results" is Very Bad, and as distros start shipping coreutils 9, this is likely to get worse, I'd like to default to returning correct files Now(tm) and debug the problem more deeply after. (Especially when, for example, someone has proposed an exclusion for using this functionality if OpenZFS is detected, and "cp workarounds for your broken feature" seems Very Bad to me.) [1] - plus some additional constraints that aren't clear to me yet, e.g. I have an all-Gentoo VM I can repro this in within 10 tries at most, and a chroot of said VM on Ubuntu 18.04 will never reproduce it, with the same root FS on both, same host, etc. |
|
Is the problem solved by setting |
|
I have not personally tried it; prior discussion in the thread said not. edit: I have now personally tried it, and it does not. |
|
@ahrens @rincebrain I've opened PR #12724 with a proposed fix for the underlying problem. It would be great if you could provide review feedback. The short version is we were not flushing dirty pages in the page cache through ->writeback before reporting data/holes, meaning we were not accounting for recent mmap(2) writes. |
|
Moot unless there are problems yet unfound that #12724 does not address. |
Unfortunately, currently sometimes SEEK_DATA/SEEK_HOLE are returning the wrong thing. So let's add a tunable to disable them for now, for people who value correctness. Suggested-by: Paul Eggert <eggert@cs.ucla.edu> Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
|
Moot again unless #12745 doesn't cure all our woes. |
Motivation and Context
#11900 is bad and not fixed, so let's degrade behavior and let people force the dangerous behavior back on if they know they need it.
Description
A tunable for making it always behave as though we've never heard of SEEK_{DATA,HOLE} before.
How Has This Been Tested?
Ran the emerge testcase that blows up in #11900 in a loop with the tunable set to 1 and 0 - with 1, it never breaks, and with 0, it breaks as usual within 10 runs...on Linux. It also passed the GH actions runs.
On FBSD, I haven't tried it at all, because I don't have a particularly good reproducer there. But since their only reproducer so far was worked around by freebsd/freebsd-src@42881526 I'm not as urgently concerned about that...
Types of changes
Checklist:
Signed-off-by.