linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] fs: Avoid to use lockdep information if it's turned off
@ 2020-11-10  1:37 Boqun Feng
  2020-11-10  1:49 ` Darrick J. Wong
  2020-11-10 15:33 ` David Sterba
  0 siblings, 2 replies; 8+ messages in thread
From: Boqun Feng @ 2020-11-10  1:37 UTC (permalink / raw)
  To: linux-kernel, linux-fsdevel
  Cc: Boqun Feng, Filipe Manana, Peter Zijlstra, Jan Kara,
	David Sterba, Nikolay Borisov, Darrick J. Wong, Alexander Viro,
	Ingo Molnar

Filipe Manana reported a warning followed by task hanging after attempts
to freeze a filesystem[1]. The problem happened in a LOCKDEP=y kernel,
and percpu_rwsem_is_held() provided incorrect results when
debug_locks == 0. Although the behavior is caused by commit 4d004099a668
("lockdep: Fix lockdep recursion"): after that lock_is_held() and its
friends always return true if debug_locks == 0. However, one could argue
that querying the lock holding information regardless if the lockdep
turn-off status is inappropriate in the first place. Therefore instead
of reverting lock_is_held() and its friends to the previous semantics,
add the explicit checking in fs code to avoid use the lock holding
information if lockdpe is turned off. And since the original problem
also happened with a silent lockdep turn-off, put a warning if
debug_locks is 0, which will help us spot the silent lockdep turn-offs.

[1]: https://lore.kernel.org/lkml/a5cf643b-842f-7a60-73c7-85d738a9276f@suse.com/

Reported-by: Filipe Manana <fdmanana@gmail.com>
Fixes: 4d004099a668 ("lockdep: Fix lockdep recursion")
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: David Sterba <dsterba@suse.com>
Cc: Nikolay Borisov <nborisov@suse.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
---
Hi Filipe,

I use the slightly different approach to fix this problem, and I think
it should have the similar effect with my previous fix[2], except that
you will hit a warning if the problem happens now. The warning is added
on purpose because I don't want to miss a silent lockdep turn-off.

Could you and other fs folks give this a try?

Regards,
Boqun

[2]: https://lore.kernel.org/lkml/20201103140828.GA2713762@boqun-archlinux/

 fs/super.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index a51c2083cd6b..1803c8d999e9 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1659,12 +1659,23 @@ int __sb_start_write(struct super_block *sb, int level, bool wait)
 	 * twice in some cases, which is OK only because we already hold a
 	 * freeze protection also on higher level. Due to these cases we have
 	 * to use wait == F (trylock mode) which must not fail.
+	 *
+	 * Note: lockdep can only prove correct information if debug_locks != 0
 	 */
 	if (wait) {
 		int i;
 
 		for (i = 0; i < level - 1; i++)
 			if (percpu_rwsem_is_held(sb->s_writers.rw_sem + i)) {
+				/*
+				 * XXX: the WARN_ON_ONCE() here is to help
+				 * track down silent lockdep turn-off, i.e.
+				 * this warning is triggered, but no lockdep
+				 * splat is reported.
+				 */
+				if (WARN_ON_ONCE(!debug_locks))
+					break;
+
 				force_trylock = true;
 				break;
 			}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-11-13 17:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10  1:37 [RFC] fs: Avoid to use lockdep information if it's turned off Boqun Feng
2020-11-10  1:49 ` Darrick J. Wong
2020-11-10  5:40   ` Boqun Feng
2020-11-10 16:49     ` Darrick J. Wong
2020-11-10 15:33 ` David Sterba
2020-11-11 14:01   ` David Sterba
2020-11-12  3:22     ` Boqun Feng
2020-11-13 17:19       ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).