This repository has been archived by the owner on Feb 26, 2020. It is now read-only.
Mutex fstrans #435
Closed
Mutex fstrans #435
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
This patch only addresses the issues identified by the style checker in mutex.h. It contains no functional changes. Signed-off-by: Brian Behlendorf <[email protected]>
To minimize the size of a kmutex_t a MUTEX_OWNER check was added. It allowed the kmutex_t wrapper to leverage the mutex owner which was already stored in the mutex for certain kernel configurations. The upside to this was that it reduced the size of the kmutex_t wrapper structure by the size of a task_struct pointer (4/8 bytes). The downside was that two mutex implementations needed to be maintained. Depending on your exact kernel configuration the correct one would be selected. Over the years this solution worked but it could be fragile since it depending heavily on assumed kernel mutex implementation details. For example the SPL_AC_MUTEX_OWNER_TASK_STRUCT configure check needed to be added when the kernel changed how the owner was stored. It also made the code more complicated than it needed to be. Therefore in the name of simplicity and portability this optimization is being retired. It will slightly increase the memory requirements for a kmutex_t but only very slightly. Signed-off-by: Brian Behlendorf <[email protected]>
There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by adding a new mutex type which is responsible for managing PF_FSTRANS. This lets us make a a small change to the ZFS source where the mutex is initialized and then be certain that all future use of that mutex will be safe. Signed-off-by: Brian Behlendorf <[email protected]>
Slightly increasing the size of a kmutex_t has caused us to exceed the stack frame warning size in splat_taskq_test2_impl(). To address this the tq_args have been moved to the heap. cc1: warnings being treated as errors spl-0.6.3/module/splat/splat-taskq.c:358: error: the frame size of 1040 bytes is larger than 1024 bytes Signed-off-by: Brian Behlendorf <[email protected]>
behlendorf
force-pushed
the
mutex-fstrans
branch
from
a352a1e to
344e28a
Compare
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 2, 2015
There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by adding a new mutex type which is responsible for managing PF_FSTRANS. This lets us make a a small change to the ZFS source where the mutex is initialized and then be certain that all future use of that mutex will be safe. NOTES: The ht_locks are no longer aligned on 64-byte boundaries. We've never studied if this is actually critical for performance when there are a large number of hash buckets. The dbuf hash has never made this optimization. Signed-off-by: Brian Behlendorf [email protected] Issue #3050 Issue #3055
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 3, 2015
There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by adding a new mutex type which is responsible for managing PF_FSTRANS. This lets us make a a small change to the ZFS source where the mutex is initialized and then be certain that all future use of that mutex will be safe. NOTES: The ht_locks are no longer aligned on 64-byte boundaries. We've never studied if this is actually critical for performance when there are a large number of hash buckets. The dbuf hash has never made this optimization. Signed-off-by: Brian Behlendorf [email protected] Issue #3050 Issue #3055
This was referenced Mar 3, 2015
behlendorf
added a commit
that referenced
this pull request
Mar 3, 2015
This patch only addresses the issues identified by the style checker in mutex.h. It contains no functional changes. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue #435
behlendorf
added a commit
that referenced
this pull request
Mar 3, 2015
To minimize the size of a kmutex_t a MUTEX_OWNER check was added. It allowed the kmutex_t wrapper to leverage the mutex owner which was already stored in the mutex for certain kernel configurations. The upside to this was that it reduced the size of the kmutex_t wrapper structure by the size of a task_struct pointer (4/8 bytes). The downside was that two mutex implementations needed to be maintained. Depending on your exact kernel configuration the correct one would be selected. Over the years this solution worked but it could be fragile since it depending heavily on assumed kernel mutex implementation details. For example the SPL_AC_MUTEX_OWNER_TASK_STRUCT configure check needed to be added when the kernel changed how the owner was stored. It also made the code more complicated than it needed to be. Therefore, in the name of simplicity and portability this optimization is being retired. It will slightly increase the memory requirements for a kmutex_t but only very slightly. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue #435
behlendorf
added a commit
that referenced
this pull request
Mar 3, 2015
There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by adding a new mutex type which is responsible for managing PF_FSTRANS, support for which was added to the SPL in commit 9099312 - Merge branch 'kmem-rework'. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue #435
behlendorf
added a commit
that referenced
this pull request
Mar 3, 2015
Slightly increasing the size of a kmutex_t has caused us to exceed the stack frame warning size in splat_taskq_test2_impl(). To address this the tq_args have been moved to the heap. cc1: warnings being treated as errors spl-0.6.3/module/splat/splat-taskq.c:358: error: the frame size of 1040 bytes is larger than 1024 bytes Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue #435
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 3, 2015
This patch only addresses the issues identified by the style checker in mutex.h. It contains no functional changes. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue openzfs#435
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 3, 2015
To minimize the size of a kmutex_t a MUTEX_OWNER check was added. It allowed the kmutex_t wrapper to leverage the mutex owner which was already stored in the mutex for certain kernel configurations. The upside to this was that it reduced the size of the kmutex_t wrapper structure by the size of a task_struct pointer (4/8 bytes). The downside was that two mutex implementations needed to be maintained. Depending on your exact kernel configuration the correct one would be selected. Over the years this solution worked but it could be fragile since it depending heavily on assumed kernel mutex implementation details. For example the SPL_AC_MUTEX_OWNER_TASK_STRUCT configure check needed to be added when the kernel changed how the owner was stored. It also made the code more complicated than it needed to be. Therefore, in the name of simplicity and portability this optimization is being retired. It will slightly increase the memory requirements for a kmutex_t but only very slightly. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue openzfs#435
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 3, 2015
There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by adding a new mutex type which is responsible for managing PF_FSTRANS, support for which was added to the SPL in commit 9099312 - Merge branch 'kmem-rework'. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue openzfs#435
kernelOfTruth
pushed a commit
to kernelOfTruth/spl
that referenced
this pull request
Mar 3, 2015
Slightly increasing the size of a kmutex_t has caused us to exceed the stack frame warning size in splat_taskq_test2_impl(). To address this the tq_args have been moved to the heap. cc1: warnings being treated as errors spl-0.6.3/module/splat/splat-taskq.c:358: error: the frame size of 1040 bytes is larger than 1024 bytes Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Issue openzfs#435
|
merged into upstream: openzfs/zfs#3132 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
No description provided.