This repository has been archived by the owner on Feb 26, 2020. It is now read-only.
add spin_lock_irqsave_nolockdep and mutex type MUTEX_NOLOCKDEP #480
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
|
Lockdep reported potential issues with five locks. Here is the output for each, for investigation. |
ofaaland
force-pushed
the
crada_nolockdep_try_3
branch
from
48cb536
to
ee6fda0
Compare
ofaaland
force-pushed
the
crada_nolockdep_try_3
branch
3 times, most recently
from
a3f5cc4
to
97cdcd9
Compare
ofaaland
force-pushed
the
crada_nolockdep_try_3
branch
5 times, most recently
from
b87a44d
to
d3f9fd4
Compare
When running a kernel with CONFIG_LOCKDEP=y, lockdep reports possible recursive locking in some cases and possible circular locking dependency in others, within the SPL and ZFS modules. When lockdep detects these conditions, it disables further lock analysis for all locks. This causes /proc/lock_stats not to reflect full information about lock contention, even in locks without dependency issues. This commit creates a new type of mutex, MUTEX_NOLOCKDEP. This mutex type causes subsequent attempts to take or release those locks to be wrapped in lockdep_off() and lockdep_on(). This commit also creates an RW_NOLOCKDEP type analagous to MUTEX_NOLOCKDEP. MUTEX_NOLOCKDEP and RW_NOLOCKDEP are also defined in zfs, in a commit to that repo, for userspace builds.
spl_inode_{lock,unlock} are triggering possible recursive locking
warnings from lockdep. The warning is a false positive.
The lock is used to protect a parent directory during delete/add
operations, used in zfs when writing/removing the cache file. The inode
lock is taken on both the parent inode and the file inode.
VFS provides an enum to subclass the lock. This patch changes the
spin_lock call to _nested version and uses the provided enum.
When taskq_dispatch() calls taskq_thread_spawn() to create a new thread for a taskq, linux lockdep warns of possible recursive locking. This is a false positive. One such call chain is as follows, when a taskq needs more threads: taskq_dispatch->taskq_thread_spawn->taskq_dispatch The initial taskq_dispatch() holds tq_lock on the taskq that needed more worker threads. The later call into taskq_dispatch() takes dynamic_taskq->tq_lock. Without subclassing, lockdep believes these could potentially be the same lock and complains. A similar case occurs when taskq_dispatch() then calls task_alloc(). This patch uses spin_lock_irqsave_nested() when taking tq_lock, with one of two new lock subclasses: subclass taskq TQ_LOCK_DYNAMIC dynamic_taskq TQ_LOCK_GENERAL any other
ofaaland
force-pushed
the
crada_nolockdep_try_3
branch
from
d3f9fd4
to
aca2626
Compare
behlendorf
pushed a commit
that referenced
this pull request
Dec 12, 2015
When running a kernel with CONFIG_LOCKDEP=y, lockdep reports possible recursive locking in some cases and possible circular locking dependency in others, within the SPL and ZFS modules. When lockdep detects these conditions, it disables further lock analysis for all locks. This causes /proc/lock_stats not to reflect full information about lock contention, even in locks without dependency issues. This commit creates a new type of mutex, MUTEX_NOLOCKDEP. This mutex type causes subsequent attempts to take or release those locks to be wrapped in lockdep_off() and lockdep_on(). This commit also creates an RW_NOLOCKDEP type analagous to MUTEX_NOLOCKDEP. MUTEX_NOLOCKDEP and RW_NOLOCKDEP are also defined in zfs, in a commit to that repo, for userspace builds. Signed-off-by: Olaf Faaland <faaland1@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #480
behlendorf
pushed a commit
that referenced
this pull request
Dec 12, 2015
spl_inode_{lock,unlock} are triggering possible recursive locking
warnings from lockdep. The warning is a false positive.
The lock is used to protect a parent directory during delete/add
operations, used in zfs when writing/removing the cache file. The inode
lock is taken on both the parent inode and the file inode.
VFS provides an enum to subclass the lock. This patch changes the
spin_lock call to _nested version and uses the provided enum.
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #480
behlendorf
pushed a commit
that referenced
this pull request
Dec 12, 2015
When taskq_dispatch() calls taskq_thread_spawn() to create a new thread for a taskq, linux lockdep warns of possible recursive locking. This is a false positive. One such call chain is as follows, when a taskq needs more threads: taskq_dispatch->taskq_thread_spawn->taskq_dispatch The initial taskq_dispatch() holds tq_lock on the taskq that needed more worker threads. The later call into taskq_dispatch() takes dynamic_taskq->tq_lock. Without subclassing, lockdep believes these could potentially be the same lock and complains. A similar case occurs when taskq_dispatch() then calls task_alloc(). This patch uses spin_lock_irqsave_nested() when taking tq_lock, with one of two new lock subclasses: subclass taskq TQ_LOCK_DYNAMIC dynamic_taskq TQ_LOCK_GENERAL any other Signed-off-by: Olaf Faaland <faaland1@llnl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #480
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.
When running a kernel with CONFIG_LOCKDEP=y, lockdep reports possible
recursive locking in some cases and possible circular locking dependency
in others, within the SPL and ZFS modules.
When lockdep detects these conditions, it disables further lock analysis
for all locks. This causes /proc/lock_stats not to reflect full
information about lock contention, even in locks without dependency
issues.
This commit creates new macros spin_lock_irqsave_nolockdep and
spin_unlock_irqrestore_nolockdep. These macros use lockdep_off() and
lockdep_on() to temporarily disable lockdep when taking/releasing the
locks.
This commit also creates a new type of mutex, MUTEX_NOLOCKDEP. This
mutex type causes subsequent attempts to take or release those locks to
be wrapped in lockdep_off() and lockdep_on().
At this time it's unknown whether this method distorts the statistics
reported by lock_stats.
This commit uses the new spin_lock macros for one such offending lock.
See also commit "Identify locks flagged by lockdep" at openzfs/zfs#3895
Combined, these two commits mark all offending locks at this time, for
later analysis and fixes or annotation.