Linux 6.12 compatibility #16582

robn · 2024-09-30T03:03:44Z

Motivation and Context

Gotta work
Gotta make that money, make purse
Got a fur coat, so I make it purr
Give 'em whiplash when they see me earn
Gotta, gotta, gotta work

Gotta make that money, make purse
Got a fur coat, so I make it purr
Gotta get that credit, get perks
Gotta, gotta, gotta work

– Work, ATEEZ, 2024.

Linux 6.12-rc1 dropped, requiring the usual smattering of updates.

Description

See commit messages.

The only possibly gnarly one is the top commit, removing the PG_error flag. I'm reasonably certain that our use has been an effective no-op since ~2017 (Linux 4.17), so this change is benign, but our mmap() magic is deeper than my ken, so I'd appreciate an expert consideration (@behlendorf? @bwatkinson?). Notes and references in the commit message.

How Has This Been Tested?

Compiled and passed basic sanity check (create, fio, export, import, scrub, unload) on kernels:

4.19.319
5.4.281
5.10.223
6.1.102
6.2.16
6.3.13
6.4.16
6.5.13
6.6.43
6.7.12
6.8.12
6.9.12
6.10.3
6.11.0
6.12-rc1

(not (yet) 5.15.167, because there's some weird build quirk happening on my test rig, unrelated to this change. will update).

ZTS run in progress, will update here when its done.

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:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which gets in the way of our our own redefinition, so we undef the macro first for our own clients. This follows what we did for kmem_cache_alloc(), see e951dba. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <robn@despairlabs.com>

behlendorf

Thanks Rob this looks good. I'll need to look more closely in to the PR_error flag removal with 6.12 but since you've left it for 6.11 and earlier there shouldn't be an issue with the supported kernel versions.

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes #16582

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes #16582

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes #16582

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes #16582

torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which gets in the way of our our own redefinition, so we undef the macro first for our own clients. This follows what we did for kmem_cache_alloc(), see e951dba. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which gets in the way of our our own redefinition, so we undef the macro first for our own clients. This follows what we did for kmem_cache_alloc(), see e951dba. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

darkbasic · 2024-11-15T17:44:05Z

@robn did you manage to run the test suite against 6.12? If so did it run cleanly?

torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which gets in the way of our our own redefinition, so we undef the macro first for our own clients. This follows what we did for kmem_cache_alloc(), see e951dba. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

robn · 2024-11-18T22:43:40Z

@darkbasic ran it overnight on 6.12.0, and yes, clean run.

darkbasic · 2024-11-20T11:37:39Z

Thanks @robn I've backported it into 2.2.x and I'm feeling confident shipping it in the Arch Linux DKMS if you managed to run the test suite against 6.12.0.

usaleem-ix · 2024-11-21T11:08:08Z

@robn since 6.12.0 is out and we have test suite running cleanly against 6.12.0, should we update META for 6.12?

behlendorf · 2024-11-21T15:43:02Z

Yes we should, #16793

torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which gets in the way of our our own redefinition, so we undef the macro first for our own clients. This follows what we did for kmem_cache_alloc(), see e951dba. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

See torvalds/linux@a2b80ce87a87. It claims the task arg is always `current`, and so it is with us, so this is a safe change to make. The only spanner is that we also support the older pre-5.17 3-arg dequeue_signal() which had different meaning, so we have to check the types to get the right one. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@641bb4394f40 asserts that this is a static flag, not intended to be variable per-file, so it moves it to file_operations instead. We just change our check to follow. No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the same commit, so there's no chance of a conflict. It's not clear to me that we need this check at all, as we never set this flag on our own files, and I can't see any way that our llseek handler could recieve a file from another filesystem. But, the whole zpl_llseek() has a number of opportunities for pleasing cleanup that are nothing to do with this change, so I'll leave that for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com> Closes openzfs#16582

robn added 5 commits September 30, 2024 09:51

behlendorf approved these changes Sep 30, 2024

View reviewed changes

behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Sep 30, 2024

behlendorf closed this in bc96b80 Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 6.12 compatibility #16582

Linux 6.12 compatibility #16582

robn commented Sep 30, 2024

behlendorf left a comment

darkbasic commented Nov 15, 2024

robn commented Nov 18, 2024

darkbasic commented Nov 20, 2024

usaleem-ix commented Nov 21, 2024

behlendorf commented Nov 21, 2024

Linux 6.12 compatibility #16582

Linux 6.12 compatibility #16582

Conversation

robn commented Sep 30, 2024

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

behlendorf left a comment

Choose a reason for hiding this comment

darkbasic commented Nov 15, 2024

robn commented Nov 18, 2024

darkbasic commented Nov 20, 2024

usaleem-ix commented Nov 21, 2024

behlendorf commented Nov 21, 2024