linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
@ 2019-08-01  1:01 Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 1/7] locking/lockdep: Add Kconfig option for bit spinlocks Thomas Gleixner
                   ` (7 more replies)
  0 siblings, 8 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

Bit spinlocks are problematic if PREEMPT_RT is enabled. They disable
preemption, which is undesired for latency reasons and breaks when regular
spinlocks are taken within the bit_spinlock locked region because regular
spinlocks are converted to 'sleeping spinlocks' on RT.

Bit spinlocks are also not covered by lock debugging, e.g. lockdep. With
the spinlock substitution in place, they can be exposed via a new config
switch: CONFIG_DEBUG_BIT_SPINLOCKS.

This series handles only buffer head and jbd2, but does not touch the
hlist_bl bit-spinlock usage. See V1 for further details:

  https://lkml.kernel.org/r/20190730112452.871257694@linutronix.de

Changes vs. V1:

  - Collected reviewed-by tags for the BH_Uptodate part

  - Reworked the JBD2 part according to Jan's review:

     - Convert state lock to a regular spinlock unconditionally

     - Refactor jbd2_journal_put_journal_head() to be RT friendly by
       restricting the bit-spinlock held section to the minimum required
       operations.

       That part is probably over-engineered, but I'm sure Jan will tell me
       sooner than later.

Thanks,

        tglx



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

* [patch V2 1/7] locking/lockdep: Add Kconfig option for bit spinlocks
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 2/7] fs/buffer: Move BH_Uptodate_Lock locking into wrapper functions Thomas Gleixner
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

Some usage sites of bit spinlocks have a substitution with regular
spinlocks which depends on CONFIG_PREEMPT_RT. But this substitution can
also be used to expose these locks to the regular lock debugging
infrastructure, e.g. lockdep.

As this increases the size of affected data structures significantly this
is guarded by a separate Kconfig switch.

Note, that only the bit spinlocks which have a substitution implemented
will be covered by this. All other bit spinlocks evade lock debugging as
before.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 lib/Kconfig.debug |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1201,6 +1201,16 @@ config DEBUG_RWSEMS
 	  This debugging feature allows mismatched rw semaphore locks
 	  and unlocks to be detected and reported.
 
+config DEBUG_BIT_SPINLOCKS
+	bool "Bit spinlock debugging"
+	depends on DEBUG_SPINLOCK
+	help
+	  This debugging feature substitutes bit spinlocks in some use
+	  cases, e.g. buffer head, zram, with with regular spinlocks so
+	  these locks are exposed to lock debugging features.
+
+	  Not all bit spinlocks are covered by this.
+
 config DEBUG_LOCK_ALLOC
 	bool "Lock debugging: detect incorrect freeing of live locks"
 	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT



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

* [patch V2 2/7] fs/buffer: Move BH_Uptodate_Lock locking into wrapper functions
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 1/7] locking/lockdep: Add Kconfig option for bit spinlocks Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 3/7] fs/buffer: Substitute BH_Uptodate_Lock for RT and bit spinlock debugging Thomas Gleixner
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

Bit spinlocks are problematic if PREEMPT_RT is enabled, because they
disable preemption, which is undesired for latency reasons and breaks when
regular spinlocks are taken within the bit_spinlock locked region because
regular spinlocks are converted to 'sleeping spinlocks' on RT. So RT
replaces the bit spinlocks with regular spinlocks to avoid this problem.

To avoid ifdeffery at the source level, wrap all BH_Uptodate_Lock bitlock
operations with inline functions, so the spinlock substitution can be done
at one place.

Using regular spinlocks can also be enabled for lock debugging purposes so
the lock operations become visible to lockdep.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
---
V2: Collected Reviewed-by tag
---
 fs/buffer.c                 |   20 ++++++--------------
 fs/ext4/page-io.c           |    6 ++----
 fs/ntfs/aops.c              |   10 +++-------
 include/linux/buffer_head.h |   16 ++++++++++++++++
 4 files changed, 27 insertions(+), 25 deletions(-)

--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -275,8 +275,7 @@ static void end_buffer_async_read(struct
 	 * decide that the page is now completely done.
 	 */
 	first = page_buffers(page);
-	local_irq_save(flags);
-	bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+	flags = bh_uptodate_lock_irqsave(first);
 	clear_buffer_async_read(bh);
 	unlock_buffer(bh);
 	tmp = bh;
@@ -289,8 +288,7 @@ static void end_buffer_async_read(struct
 		}
 		tmp = tmp->b_this_page;
 	} while (tmp != bh);
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
+	bh_uptodate_unlock_irqrestore(first, flags);
 
 	/*
 	 * If none of the buffers had errors and they are all
@@ -302,9 +300,7 @@ static void end_buffer_async_read(struct
 	return;
 
 still_busy:
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
-	return;
+	bh_uptodate_unlock_irqrestore(first, flags);
 }
 
 /*
@@ -331,8 +327,7 @@ void end_buffer_async_write(struct buffe
 	}
 
 	first = page_buffers(page);
-	local_irq_save(flags);
-	bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+	flags = bh_uptodate_lock_irqsave(first);
 
 	clear_buffer_async_write(bh);
 	unlock_buffer(bh);
@@ -344,15 +339,12 @@ void end_buffer_async_write(struct buffe
 		}
 		tmp = tmp->b_this_page;
 	}
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
+	bh_uptodate_unlock_irqrestore(first, flags);
 	end_page_writeback(page);
 	return;
 
 still_busy:
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
-	return;
+	bh_uptodate_unlock_irqrestore(first, flags);
 }
 EXPORT_SYMBOL(end_buffer_async_write);
 
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -90,8 +90,7 @@ static void ext4_finish_bio(struct bio *
 		 * We check all buffers in the page under BH_Uptodate_Lock
 		 * to avoid races with other end io clearing async_write flags
 		 */
-		local_irq_save(flags);
-		bit_spin_lock(BH_Uptodate_Lock, &head->b_state);
+		flags = bh_uptodate_lock_irqsave(head);
 		do {
 			if (bh_offset(bh) < bio_start ||
 			    bh_offset(bh) + bh->b_size > bio_end) {
@@ -103,8 +102,7 @@ static void ext4_finish_bio(struct bio *
 			if (bio->bi_status)
 				buffer_io_error(bh);
 		} while ((bh = bh->b_this_page) != head);
-		bit_spin_unlock(BH_Uptodate_Lock, &head->b_state);
-		local_irq_restore(flags);
+		bh_uptodate_unlock_irqrestore(head, flags);
 		if (!under_io) {
 			fscrypt_free_bounce_page(bounce_page);
 			end_page_writeback(page);
--- a/fs/ntfs/aops.c
+++ b/fs/ntfs/aops.c
@@ -92,8 +92,7 @@ static void ntfs_end_buffer_async_read(s
 				"0x%llx.", (unsigned long long)bh->b_blocknr);
 	}
 	first = page_buffers(page);
-	local_irq_save(flags);
-	bit_spin_lock(BH_Uptodate_Lock, &first->b_state);
+	flags = bh_uptodate_lock_irqsave(first);
 	clear_buffer_async_read(bh);
 	unlock_buffer(bh);
 	tmp = bh;
@@ -108,8 +107,7 @@ static void ntfs_end_buffer_async_read(s
 		}
 		tmp = tmp->b_this_page;
 	} while (tmp != bh);
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
+	bh_uptodate_unlock_irqrestore(first, flags);
 	/*
 	 * If none of the buffers had errors then we can set the page uptodate,
 	 * but we first have to perform the post read mst fixups, if the
@@ -142,9 +140,7 @@ static void ntfs_end_buffer_async_read(s
 	unlock_page(page);
 	return;
 still_busy:
-	bit_spin_unlock(BH_Uptodate_Lock, &first->b_state);
-	local_irq_restore(flags);
-	return;
+	bh_uptodate_unlock_irqrestore(first, flags);
 }
 
 /**
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -78,6 +78,22 @@ struct buffer_head {
 	atomic_t b_count;		/* users using this buffer_head */
 };
 
+static inline unsigned long bh_uptodate_lock_irqsave(struct buffer_head *bh)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	bit_spin_lock(BH_Uptodate_Lock, &bh->b_state);
+	return flags;
+}
+
+static inline void
+bh_uptodate_unlock_irqrestore(struct buffer_head *bh, unsigned long flags)
+{
+	bit_spin_unlock(BH_Uptodate_Lock, &bh->b_state);
+	local_irq_restore(flags);
+}
+
 /*
  * macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
  * and buffer_foo() functions.



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

* [patch V2 3/7] fs/buffer: Substitute BH_Uptodate_Lock for RT and bit spinlock debugging
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 1/7] locking/lockdep: Add Kconfig option for bit spinlocks Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 2/7] fs/buffer: Move BH_Uptodate_Lock locking into wrapper functions Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  1:01 ` [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state() Thomas Gleixner
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Alexander Viro, Matthew Wilcox, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

Bit spinlocks are problematic if PREEMPT_RT is enabled. They disable
preemption, which is undesired for latency reasons and breaks when regular
spinlocks are taken within the bit_spinlock locked region because regular
spinlocks are converted to 'sleeping spinlocks' on RT.

Substitute the BH_Uptodate_Lock bit spinlock with a regular spinlock for
PREEMPT_RT enabled kernels.

Bit spinlocks are also not covered by lock debugging, e.g. lockdep. With
the spinlock substitution in place, they can be exposed via
CONFIG_DEBUG_BIT_SPINLOCKS.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: linux-fsdevel@vger.kernel.org
---
V2: Collected Reviewed-by tag
---
 fs/buffer.c                 |    1 +
 include/linux/buffer_head.h |   31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3360,6 +3360,7 @@ struct buffer_head *alloc_buffer_head(gf
 	struct buffer_head *ret = kmem_cache_zalloc(bh_cachep, gfp_flags);
 	if (ret) {
 		INIT_LIST_HEAD(&ret->b_assoc_buffers);
+		buffer_head_init_locks(ret);
 		preempt_disable();
 		__this_cpu_inc(bh_accounting.nr);
 		recalc_bh_state();
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -76,8 +76,35 @@ struct buffer_head {
 	struct address_space *b_assoc_map;	/* mapping this buffer is
 						   associated with */
 	atomic_t b_count;		/* users using this buffer_head */
+
+#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_DEBUG_BIT_SPINLOCKS)
+	spinlock_t b_uptodate_lock;
+#endif
 };
 
+#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_DEBUG_BIT_SPINLOCKS)
+
+static inline unsigned long bh_uptodate_lock_irqsave(struct buffer_head *bh)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&bh->b_uptodate_lock, flags);
+	return flags;
+}
+
+static inline void
+bh_uptodate_unlock_irqrestore(struct buffer_head *bh, unsigned long flags)
+{
+	spin_unlock_irqrestore(&bh->b_uptodate_lock, flags);
+}
+
+static inline void buffer_head_init_locks(struct buffer_head *bh)
+{
+	spin_lock_init(&bh->b_uptodate_lock);
+}
+
+#else /* PREEMPT_RT || DEBUG_BIT_SPINLOCKS */
+
 static inline unsigned long bh_uptodate_lock_irqsave(struct buffer_head *bh)
 {
 	unsigned long flags;
@@ -94,6 +121,10 @@ bh_uptodate_unlock_irqrestore(struct buf
 	local_irq_restore(flags);
 }
 
+static inline void buffer_head_init_locks(struct buffer_head *bh) { }
+
+#endif /* !PREEMPT_RT && !DEBUG_BIT_SPINLOCKS */
+
 /*
  * macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
  * and buffer_foo() functions.



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

* [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state()
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
                   ` (2 preceding siblings ...)
  2019-08-01  1:01 ` [patch V2 3/7] fs/buffer: Substitute BH_Uptodate_Lock for RT and bit spinlock debugging Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  9:00   ` Jan Kara
  2019-08-01  1:01 ` [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer() Thomas Gleixner
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright,
	linux-ext4, Theodore Tso, Jan Kara, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

No users.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-ext4@vger.kernel.org
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Jan Kara <jack@suse.com>
---
 include/linux/jbd2.h |    5 -----
 1 file changed, 5 deletions(-)

--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -347,11 +347,6 @@ static inline void jbd_lock_bh_state(str
 	bit_spin_lock(BH_State, &bh->b_state);
 }
 
-static inline int jbd_trylock_bh_state(struct buffer_head *bh)
-{
-	return bit_spin_trylock(BH_State, &bh->b_state);
-}
-
 static inline int jbd_is_locked_bh_state(struct buffer_head *bh)
 {
 	return bit_spin_is_locked(BH_State, &bh->b_state);



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

* [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer()
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
                   ` (3 preceding siblings ...)
  2019-08-01  1:01 ` [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state() Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  9:04   ` Jan Kara
  2019-08-01  1:01 ` [patch V2 6/7] fs/jbd2: Make state lock a spinlock Thomas Gleixner
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright,
	linux-ext4, Theodore Tso, Jan Kara, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

journal_unmap_buffer() checks first whether the buffer head is a journal.
If so it takes locks and then invokes jbd2_journal_grab_journal_head()
followed by another check whether this is journal head buffer.

The double checking is pointless.

Replace the initial check with jbd2_journal_grab_journal_head() which
alredy checks whether the buffer head is actually a journal.

Allows also early access to the journal head pointer for the upcoming
conversion of state lock to a regular spinlock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-ext4@vger.kernel.org
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Jan Kara <jack@suse.com>
---
V2: New patch
---
 fs/jbd2/transaction.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -2196,7 +2196,8 @@ static int journal_unmap_buffer(journal_
 	 * holding the page lock. --sct
 	 */
 
-	if (!buffer_jbd(bh))
+	jh = jbd2_journal_grab_journal_head(bh);
+	if (!jh)
 		goto zap_buffer_unlocked;
 
 	/* OK, we have data buffer in journaled mode */
@@ -2204,10 +2205,6 @@ static int journal_unmap_buffer(journal_
 	jbd_lock_bh_state(bh);
 	spin_lock(&journal->j_list_lock);
 
-	jh = jbd2_journal_grab_journal_head(bh);
-	if (!jh)
-		goto zap_buffer_no_jh;
-
 	/*
 	 * We cannot remove the buffer from checkpoint lists until the
 	 * transaction adding inode to orphan list (let's call it T)
@@ -2329,7 +2326,6 @@ static int journal_unmap_buffer(journal_
 	 */
 	jh->b_modified = 0;
 	jbd2_journal_put_journal_head(jh);
-zap_buffer_no_jh:
 	spin_unlock(&journal->j_list_lock);
 	jbd_unlock_bh_state(bh);
 	write_unlock(&journal->j_state_lock);



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

* [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
                   ` (4 preceding siblings ...)
  2019-08-01  1:01 ` [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer() Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01 11:28   ` Peter Zijlstra
  2019-08-01 17:57   ` Jan Kara
  2019-08-01  1:01 ` [patch V2 7/7] fs/jbd2: Free journal head outside of locked region Thomas Gleixner
  2019-08-02  7:56 ` [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Christoph Hellwig
  7 siblings, 2 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Jan Kara, Matthew Wilcox, Alexander Viro, linux-fsdevel

Bit-spinlocks are problematic on PREEMPT_RT if functions which might sleep
on RT, e.g. spin_lock(), alloc/free(), are invoked inside the lock held
region because bit spinlocks disable preemption even on RT.

A first attempt was to replace state lock with a spinlock placed in struct
buffer_head and make the locking conditional on PREEMPT_RT and
DEBUG_BIT_SPINLOCKS.

Jan pointed out that there is a 4 byte hole in struct journal_head where a
regular spinlock fits in and he would not object to convert the state lock
to a spinlock unconditionally.

Aside of solving the RT problem, this also gains lockdep coverage for the
journal head state lock (bit-spinlocks are not covered by lockdep as it's
hard to fit a lockdep map into a single bit).

The trivial change would have been to convert the jbd_*lock_bh_state()
inlines, but that comes with the downside that these functions take a
buffer head pointer which needs to be converted to a journal head pointer
which adds another level of indirection.

As almost all functions which use this lock have a journal head pointer
readily available, it makes more sense to remove the lock helper inlines
and write out spin_*lock() at all call sites.

Fixup all locking comments as well.

Suggested-by: Jan Kara <jack@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Jan Kara <jack@suse.com>
Cc: linux-ext4@vger.kernel.org
---
V2: New patch
---
 fs/jbd2/commit.c             |    8 ++--
 fs/jbd2/journal.c            |   10 +++--
 fs/jbd2/transaction.c        |   83 +++++++++++++++++++++----------------------
 fs/ocfs2/suballoc.c          |   19 +++++----
 include/linux/jbd2.h         |   20 +---------
 include/linux/journal-head.h |   19 ++++++---
 6 files changed, 77 insertions(+), 82 deletions(-)

--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -482,10 +482,10 @@ void jbd2_journal_commit_transaction(jou
 		if (jh->b_committed_data) {
 			struct buffer_head *bh = jh2bh(jh);
 
-			jbd_lock_bh_state(bh);
+			spin_lock(&jh->state_lock);
 			jbd2_free(jh->b_committed_data, bh->b_size);
 			jh->b_committed_data = NULL;
-			jbd_unlock_bh_state(bh);
+			spin_unlock(&jh->state_lock);
 		}
 		jbd2_journal_refile_buffer(journal, jh);
 	}
@@ -927,7 +927,7 @@ void jbd2_journal_commit_transaction(jou
 		 * done with it.
 		 */
 		get_bh(bh);
-		jbd_lock_bh_state(bh);
+		spin_lock(&jh->state_lock);
 		J_ASSERT_JH(jh,	jh->b_transaction == commit_transaction);
 
 		/*
@@ -1023,7 +1023,7 @@ void jbd2_journal_commit_transaction(jou
 		}
 		JBUFFER_TRACE(jh, "refile or unfile buffer");
 		__jbd2_journal_refile_buffer(jh);
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 		if (try_to_free)
 			release_buffer_page(bh);	/* Drops bh reference */
 		else
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -365,7 +365,7 @@ int jbd2_journal_write_metadata_buffer(t
 	/* keep subsequent assertions sane */
 	atomic_set(&new_bh->b_count, 1);
 
-	jbd_lock_bh_state(bh_in);
+	spin_lock(&jh_in->state_lock);
 repeat:
 	/*
 	 * If a new transaction has already done a buffer copy-out, then
@@ -407,13 +407,13 @@ int jbd2_journal_write_metadata_buffer(t
 	if (need_copy_out && !done_copy_out) {
 		char *tmp;
 
-		jbd_unlock_bh_state(bh_in);
+		spin_unlock(&jh_in->state_lock);
 		tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
 		if (!tmp) {
 			brelse(new_bh);
 			return -ENOMEM;
 		}
-		jbd_lock_bh_state(bh_in);
+		spin_lock(&jh_in->state_lock);
 		if (jh_in->b_frozen_data) {
 			jbd2_free(tmp, bh_in->b_size);
 			goto repeat;
@@ -466,7 +466,7 @@ int jbd2_journal_write_metadata_buffer(t
 	__jbd2_journal_file_buffer(jh_in, transaction, BJ_Shadow);
 	spin_unlock(&journal->j_list_lock);
 	set_buffer_shadow(bh_in);
-	jbd_unlock_bh_state(bh_in);
+	spin_unlock(&jh_in->state_lock);
 
 	return do_escape | (done_copy_out << 1);
 }
@@ -2412,6 +2412,8 @@ static struct journal_head *journal_allo
 		ret = kmem_cache_zalloc(jbd2_journal_head_cache,
 				GFP_NOFS | __GFP_NOFAIL);
 	}
+	if (ret)
+		spin_lock_init(&ret->state_lock);
 	return ret;
 }
 
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -876,7 +876,7 @@ do_get_write_access(handle_t *handle, st
 
  	start_lock = jiffies;
 	lock_buffer(bh);
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 
 	/* If it takes too long to lock the buffer, trace it */
 	time_lock = jbd2_time_diff(start_lock, jiffies);
@@ -926,7 +926,7 @@ do_get_write_access(handle_t *handle, st
 
 	error = -EROFS;
 	if (is_handle_aborted(handle)) {
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 		goto out;
 	}
 	error = 0;
@@ -990,7 +990,7 @@ do_get_write_access(handle_t *handle, st
 	 */
 	if (buffer_shadow(bh)) {
 		JBUFFER_TRACE(jh, "on shadow: sleep");
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 		wait_on_bit_io(&bh->b_state, BH_Shadow, TASK_UNINTERRUPTIBLE);
 		goto repeat;
 	}
@@ -1011,7 +1011,7 @@ do_get_write_access(handle_t *handle, st
 		JBUFFER_TRACE(jh, "generate frozen data");
 		if (!frozen_buffer) {
 			JBUFFER_TRACE(jh, "allocate memory for buffer");
-			jbd_unlock_bh_state(bh);
+			spin_unlock(&jh->state_lock);
 			frozen_buffer = jbd2_alloc(jh2bh(jh)->b_size,
 						   GFP_NOFS | __GFP_NOFAIL);
 			goto repeat;
@@ -1030,7 +1030,7 @@ do_get_write_access(handle_t *handle, st
 	jh->b_next_transaction = transaction;
 
 done:
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 
 	/*
 	 * If we are about to journal a buffer, then any revoke pending on it is
@@ -1169,7 +1169,7 @@ int jbd2_journal_get_create_access(handl
 	 * that case: the transaction must have deleted the buffer for it to be
 	 * reused here.
 	 */
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 	J_ASSERT_JH(jh, (jh->b_transaction == transaction ||
 		jh->b_transaction == NULL ||
 		(jh->b_transaction == journal->j_committing_transaction &&
@@ -1204,7 +1204,7 @@ int jbd2_journal_get_create_access(handl
 		jh->b_next_transaction = transaction;
 		spin_unlock(&journal->j_list_lock);
 	}
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 
 	/*
 	 * akpm: I added this.  ext3_alloc_branch can pick up new indirect
@@ -1272,13 +1272,13 @@ int jbd2_journal_get_undo_access(handle_
 		committed_data = jbd2_alloc(jh2bh(jh)->b_size,
 					    GFP_NOFS|__GFP_NOFAIL);
 
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 	if (!jh->b_committed_data) {
 		/* Copy out the current buffer contents into the
 		 * preserved, committed copy. */
 		JBUFFER_TRACE(jh, "generate b_committed data");
 		if (!committed_data) {
-			jbd_unlock_bh_state(bh);
+			spin_unlock(&jh->state_lock);
 			goto repeat;
 		}
 
@@ -1286,7 +1286,7 @@ int jbd2_journal_get_undo_access(handle_
 		committed_data = NULL;
 		memcpy(jh->b_committed_data, bh->b_data, bh->b_size);
 	}
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 out:
 	jbd2_journal_put_journal_head(jh);
 	if (unlikely(committed_data))
@@ -1387,16 +1387,16 @@ int jbd2_journal_dirty_metadata(handle_t
 	 */
 	if (jh->b_transaction != transaction &&
 	    jh->b_next_transaction != transaction) {
-		jbd_lock_bh_state(bh);
+		spin_lock(&jh->state_lock);
 		J_ASSERT_JH(jh, jh->b_transaction == transaction ||
 				jh->b_next_transaction == transaction);
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 	}
 	if (jh->b_modified == 1) {
 		/* If it's in our transaction it must be in BJ_Metadata list. */
 		if (jh->b_transaction == transaction &&
 		    jh->b_jlist != BJ_Metadata) {
-			jbd_lock_bh_state(bh);
+			spin_lock(&jh->state_lock);
 			if (jh->b_transaction == transaction &&
 			    jh->b_jlist != BJ_Metadata)
 				pr_err("JBD2: assertion failure: h_type=%u "
@@ -1406,13 +1406,13 @@ int jbd2_journal_dirty_metadata(handle_t
 				       jh->b_jlist);
 			J_ASSERT_JH(jh, jh->b_transaction != transaction ||
 					jh->b_jlist == BJ_Metadata);
-			jbd_unlock_bh_state(bh);
+			spin_unlock(&jh->state_lock);
 		}
 		goto out;
 	}
 
 	journal = transaction->t_journal;
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 
 	if (jh->b_modified == 0) {
 		/*
@@ -1498,7 +1498,7 @@ int jbd2_journal_dirty_metadata(handle_t
 	__jbd2_journal_file_buffer(jh, transaction, BJ_Metadata);
 	spin_unlock(&journal->j_list_lock);
 out_unlock_bh:
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 out:
 	JBUFFER_TRACE(jh, "exit");
 	return ret;
@@ -1536,18 +1536,18 @@ int jbd2_journal_forget (handle_t *handl
 
 	BUFFER_TRACE(bh, "entry");
 
-	jbd_lock_bh_state(bh);
-
 	if (!buffer_jbd(bh))
 		goto not_jbd;
+
 	jh = bh2jh(bh);
+	spin_lock(&jh->state_lock);
 
 	/* Critical error: attempting to delete a bitmap buffer, maybe?
 	 * Don't do any jbd operations, and return an error. */
 	if (!J_EXPECT_JH(jh, !jh->b_committed_data,
 			 "inconsistent data on disk")) {
 		err = -EIO;
-		goto not_jbd;
+		goto bad_jbd;
 	}
 
 	/* keep track of whether or not this transaction modified us */
@@ -1664,7 +1664,7 @@ int jbd2_journal_forget (handle_t *handl
 		spin_unlock(&journal->j_list_lock);
 	}
 
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 	__brelse(bh);
 drop:
 	if (drop_reserve) {
@@ -1673,8 +1673,9 @@ int jbd2_journal_forget (handle_t *handl
 	}
 	return err;
 
+bad_jbd:
+	spin_unlock(&jh->state_lock);
 not_jbd:
-	jbd_unlock_bh_state(bh);
 	__bforget(bh);
 	goto drop;
 }
@@ -1875,7 +1876,7 @@ int jbd2_journal_stop(handle_t *handle)
  *
  * j_list_lock is held.
  *
- * jbd_lock_bh_state(jh2bh(jh)) is held.
+ * jh->state_lock is held.
  */
 
 static inline void
@@ -1899,7 +1900,7 @@ static inline void
  *
  * Called with j_list_lock held, and the journal may not be locked.
  *
- * jbd_lock_bh_state(jh2bh(jh)) is held.
+ * jh->state_lock is held.
  */
 
 static inline void
@@ -1931,7 +1932,7 @@ static void __jbd2_journal_temp_unlink_b
 	transaction_t *transaction;
 	struct buffer_head *bh = jh2bh(jh);
 
-	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
+	assert_spin_locked(&jh->state_lock);
 	transaction = jh->b_transaction;
 	if (transaction)
 		assert_spin_locked(&transaction->t_journal->j_list_lock);
@@ -1987,18 +1988,18 @@ void jbd2_journal_unfile_buffer(journal_
 
 	/* Get reference so that buffer cannot be freed before we unlock it */
 	get_bh(bh);
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 	spin_lock(&journal->j_list_lock);
 	__jbd2_journal_unfile_buffer(jh);
 	spin_unlock(&journal->j_list_lock);
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 	__brelse(bh);
 }
 
 /*
  * Called from jbd2_journal_try_to_free_buffers().
  *
- * Called under jbd_lock_bh_state(bh)
+ * Called under spin_lock(&jh->state_lock)
  */
 static void
 __journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh)
@@ -2085,10 +2086,10 @@ int jbd2_journal_try_to_free_buffers(jou
 		if (!jh)
 			continue;
 
-		jbd_lock_bh_state(bh);
+		spin_lock(&jh->state_lock);
 		__journal_try_to_free_buffer(journal, bh);
 		jbd2_journal_put_journal_head(jh);
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 		if (buffer_jbd(bh))
 			goto busy;
 	} while ((bh = bh->b_this_page) != head);
@@ -2109,7 +2110,7 @@ int jbd2_journal_try_to_free_buffers(jou
  *
  * Called under j_list_lock.
  *
- * Called under jbd_lock_bh_state(bh).
+ * Called under spin_lock(&jh->state_lock).
  */
 static int __dispose_buffer(struct journal_head *jh, transaction_t *transaction)
 {
@@ -2202,7 +2203,7 @@ static int journal_unmap_buffer(journal_
 
 	/* OK, we have data buffer in journaled mode */
 	write_lock(&journal->j_state_lock);
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 	spin_lock(&journal->j_list_lock);
 
 	/*
@@ -2285,7 +2286,7 @@ static int journal_unmap_buffer(journal_
 		if (partial_page) {
 			jbd2_journal_put_journal_head(jh);
 			spin_unlock(&journal->j_list_lock);
-			jbd_unlock_bh_state(bh);
+			spin_unlock(&jh->state_lock);
 			write_unlock(&journal->j_state_lock);
 			return -EBUSY;
 		}
@@ -2300,7 +2301,7 @@ static int journal_unmap_buffer(journal_
 			jh->b_next_transaction = journal->j_running_transaction;
 		jbd2_journal_put_journal_head(jh);
 		spin_unlock(&journal->j_list_lock);
-		jbd_unlock_bh_state(bh);
+		spin_unlock(&jh->state_lock);
 		write_unlock(&journal->j_state_lock);
 		return 0;
 	} else {
@@ -2327,7 +2328,7 @@ static int journal_unmap_buffer(journal_
 	jh->b_modified = 0;
 	jbd2_journal_put_journal_head(jh);
 	spin_unlock(&journal->j_list_lock);
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 	write_unlock(&journal->j_state_lock);
 zap_buffer_unlocked:
 	clear_buffer_dirty(bh);
@@ -2415,7 +2416,7 @@ void __jbd2_journal_file_buffer(struct j
 	int was_dirty = 0;
 	struct buffer_head *bh = jh2bh(jh);
 
-	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
+	assert_spin_locked(&jh->state_lock);
 	assert_spin_locked(&transaction->t_journal->j_list_lock);
 
 	J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);
@@ -2477,11 +2478,11 @@ void __jbd2_journal_file_buffer(struct j
 void jbd2_journal_file_buffer(struct journal_head *jh,
 				transaction_t *transaction, int jlist)
 {
-	jbd_lock_bh_state(jh2bh(jh));
+	spin_lock(&jh->state_lock);
 	spin_lock(&transaction->t_journal->j_list_lock);
 	__jbd2_journal_file_buffer(jh, transaction, jlist);
 	spin_unlock(&transaction->t_journal->j_list_lock);
-	jbd_unlock_bh_state(jh2bh(jh));
+	spin_unlock(&jh->state_lock);
 }
 
 /*
@@ -2491,7 +2492,7 @@ void jbd2_journal_file_buffer(struct jou
  * buffer on that transaction's metadata list.
  *
  * Called under j_list_lock
- * Called under jbd_lock_bh_state(jh2bh(jh))
+ * Called under jh->state_lock
  *
  * jh and bh may be already free when this function returns
  */
@@ -2500,7 +2501,7 @@ void __jbd2_journal_refile_buffer(struct
 	int was_dirty, jlist;
 	struct buffer_head *bh = jh2bh(jh);
 
-	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
+	assert_spin_locked(&jh->state_lock);
 	if (jh->b_transaction)
 		assert_spin_locked(&jh->b_transaction->t_journal->j_list_lock);
 
@@ -2549,10 +2550,10 @@ void jbd2_journal_refile_buffer(journal_
 
 	/* Get reference so that buffer cannot be freed before we unlock it */
 	get_bh(bh);
-	jbd_lock_bh_state(bh);
+	spin_lock(&jh->state_lock);
 	spin_lock(&journal->j_list_lock);
 	__jbd2_journal_refile_buffer(jh);
-	jbd_unlock_bh_state(bh);
+	spin_unlock(&jh->state_lock);
 	spin_unlock(&journal->j_list_lock);
 	__brelse(bh);
 }
--- a/fs/ocfs2/suballoc.c
+++ b/fs/ocfs2/suballoc.c
@@ -1252,6 +1252,7 @@ static int ocfs2_test_bg_bit_allocatable
 					 int nr)
 {
 	struct ocfs2_group_desc *bg = (struct ocfs2_group_desc *) bg_bh->b_data;
+	struct journal_head *jh;
 	int ret;
 
 	if (ocfs2_test_bit(nr, (unsigned long *)bg->bg_bitmap))
@@ -1260,13 +1261,14 @@ static int ocfs2_test_bg_bit_allocatable
 	if (!buffer_jbd(bg_bh))
 		return 1;
 
-	jbd_lock_bh_state(bg_bh);
-	bg = (struct ocfs2_group_desc *) bh2jh(bg_bh)->b_committed_data;
+	jh = bh2jh(bg_bh);
+	spin_lock(&jh->state_lock);
+	bg = (struct ocfs2_group_desc *) jh->b_committed_data;
 	if (bg)
 		ret = !ocfs2_test_bit(nr, (unsigned long *)bg->bg_bitmap);
 	else
 		ret = 1;
-	jbd_unlock_bh_state(bg_bh);
+	spin_unlock(&jh->state_lock);
 
 	return ret;
 }
@@ -2387,6 +2389,7 @@ static int ocfs2_block_group_clear_bits(
 	int status;
 	unsigned int tmp;
 	struct ocfs2_group_desc *undo_bg = NULL;
+	struct journal_head *jh;
 
 	/* The caller got this descriptor from
 	 * ocfs2_read_group_descriptor().  Any corruption is a code bug. */
@@ -2405,10 +2408,10 @@ static int ocfs2_block_group_clear_bits(
 		goto bail;
 	}
 
+	jh = bh2jh(group_bh);
 	if (undo_fn) {
-		jbd_lock_bh_state(group_bh);
-		undo_bg = (struct ocfs2_group_desc *)
-					bh2jh(group_bh)->b_committed_data;
+		spin_lock(&jh->state_lock);
+		undo_bg = (struct ocfs2_group_desc *) jh->b_committed_data;
 		BUG_ON(!undo_bg);
 	}
 
@@ -2423,7 +2426,7 @@ static int ocfs2_block_group_clear_bits(
 	le16_add_cpu(&bg->bg_free_bits_count, num_bits);
 	if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) {
 		if (undo_fn)
-			jbd_unlock_bh_state(group_bh);
+			spin_unlock(&jh->state_lock);
 		return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit count %u but claims %u are freed. num_bits %d\n",
 				   (unsigned long long)le64_to_cpu(bg->bg_blkno),
 				   le16_to_cpu(bg->bg_bits),
@@ -2432,7 +2435,7 @@ static int ocfs2_block_group_clear_bits(
 	}
 
 	if (undo_fn)
-		jbd_unlock_bh_state(group_bh);
+		spin_unlock(&jh->state_lock);
 
 	ocfs2_journal_dirty(handle, group_bh);
 bail:
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -313,7 +313,6 @@ enum jbd_state_bits {
 	BH_Revoked,		/* Has been revoked from the log */
 	BH_RevokeValid,		/* Revoked flag is valid */
 	BH_JBDDirty,		/* Is dirty but journaled */
-	BH_State,		/* Pins most journal_head state */
 	BH_JournalHead,		/* Pins bh->b_private and jh->b_bh */
 	BH_Shadow,		/* IO on shadow buffer is running */
 	BH_Verified,		/* Metadata block has been verified ok */
@@ -342,21 +341,6 @@ static inline struct journal_head *bh2jh
 	return bh->b_private;
 }
 
-static inline void jbd_lock_bh_state(struct buffer_head *bh)
-{
-	bit_spin_lock(BH_State, &bh->b_state);
-}
-
-static inline int jbd_is_locked_bh_state(struct buffer_head *bh)
-{
-	return bit_spin_is_locked(BH_State, &bh->b_state);
-}
-
-static inline void jbd_unlock_bh_state(struct buffer_head *bh)
-{
-	bit_spin_unlock(BH_State, &bh->b_state);
-}
-
 static inline void jbd_lock_bh_journal_head(struct buffer_head *bh)
 {
 	bit_spin_lock(BH_JournalHead, &bh->b_state);
@@ -551,9 +535,9 @@ struct transaction_chp_stats_s {
  *      ->jbd_lock_bh_journal_head()	(This is "innermost")
  *
  *    j_state_lock
- *    ->jbd_lock_bh_state()
+ *    ->jh->state_lock
  *
- *    jbd_lock_bh_state()
+ *    jh->state_lock
  *    ->j_list_lock
  *
  *    j_state_lock
--- a/include/linux/journal-head.h
+++ b/include/linux/journal-head.h
@@ -24,13 +24,18 @@ struct journal_head {
 	struct buffer_head *b_bh;
 
 	/*
+	 * Protect the buffer head state
+	 */
+	spinlock_t state_lock;
+
+	/*
 	 * Reference count - see description in journal.c
 	 * [jbd_lock_bh_journal_head()]
 	 */
 	int b_jcount;
 
 	/*
-	 * Journalling list for this buffer [jbd_lock_bh_state()]
+	 * Journalling list for this buffer [jh->state_lock]
 	 * NOTE: We *cannot* combine this with b_modified into a bitfield
 	 * as gcc would then (which the C standard allows but which is
 	 * very unuseful) make 64-bit accesses to the bitfield and clobber
@@ -41,20 +46,20 @@ struct journal_head {
 	/*
 	 * This flag signals the buffer has been modified by
 	 * the currently running transaction
-	 * [jbd_lock_bh_state()]
+	 * [jh->state_lock]
 	 */
 	unsigned b_modified;
 
 	/*
 	 * Copy of the buffer data frozen for writing to the log.
-	 * [jbd_lock_bh_state()]
+	 * [jh->state_lock]
 	 */
 	char *b_frozen_data;
 
 	/*
 	 * Pointer to a saved copy of the buffer containing no uncommitted
 	 * deallocation references, so that allocations can avoid overwriting
-	 * uncommitted deletes. [jbd_lock_bh_state()]
+	 * uncommitted deletes. [jh->state_lock]
 	 */
 	char *b_committed_data;
 
@@ -63,7 +68,7 @@ struct journal_head {
 	 * metadata: either the running transaction or the committing
 	 * transaction (if there is one).  Only applies to buffers on a
 	 * transaction's data or metadata journaling list.
-	 * [j_list_lock] [jbd_lock_bh_state()]
+	 * [j_list_lock] [jh->state_lock]
 	 * Either of these locks is enough for reading, both are needed for
 	 * changes.
 	 */
@@ -73,13 +78,13 @@ struct journal_head {
 	 * Pointer to the running compound transaction which is currently
 	 * modifying the buffer's metadata, if there was already a transaction
 	 * committing it when the new transaction touched it.
-	 * [t_list_lock] [jbd_lock_bh_state()]
+	 * [t_list_lock] [jh->state_lock]
 	 */
 	transaction_t *b_next_transaction;
 
 	/*
 	 * Doubly-linked list of buffers on a transaction's data, metadata or
-	 * forget queue. [t_list_lock] [jbd_lock_bh_state()]
+	 * forget queue. [t_list_lock] [jh->state_lock]
 	 */
 	struct journal_head *b_tnext, *b_tprev;
 



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

* [patch V2 7/7] fs/jbd2: Free journal head outside of locked region
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
                   ` (5 preceding siblings ...)
  2019-08-01  1:01 ` [patch V2 6/7] fs/jbd2: Make state lock a spinlock Thomas Gleixner
@ 2019-08-01  1:01 ` Thomas Gleixner
  2019-08-01  9:22   ` Jan Kara
  2019-08-02  7:56 ` [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Christoph Hellwig
  7 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01  1:01 UTC (permalink / raw)
  To: LKML
  Cc: Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	linux-ext4, Theodore Tso, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

On PREEMPT_RT bit-spinlocks have the same semantics as on PREEMPT_RT=n,
i.e. they disable preemption. That means functions which are not safe to be
called in preempt disabled context on RT trigger a might_sleep() assert.

The journal head bit spinlock is mostly held for short code sequences with
trivial RT safe functionality, except for one place:

jbd2_journal_put_journal_head() invokes __journal_remove_journal_head()
with the journal head bit spinlock held. __journal_remove_journal_head()
invokes kmem_cache_free() which must not be called with preemption disabled
on RT.

Jan suggested to rework the removal function so the actual free happens
outside the bit-spinlocked region.

Split it into two parts:

  - Do the sanity checks and the buffer head detach under the lock

  - Do the actual free after dropping the lock

There is error case handling in the free part which needs to dereference
the b_size field of the now detached buffer head. Due to paranoia (caused
by ignorance) the size is retrieved in the detach function and handed into
the free function. Might be over-engineered, but better safe than sorry.

This makes the journal head bit-spinlock usage RT compliant and also avoids
nested locking which is not covered by lockdep.

Suggested-by: Jan Kara <jack@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-ext4@vger.kernel.org
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Jan Kara <jack@suse.com>
---
V2: New patch
---
 fs/jbd2/journal.c |   28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2521,9 +2521,10 @@ struct journal_head *jbd2_journal_grab_j
 	return jh;
 }
 
-static void __journal_remove_journal_head(struct buffer_head *bh)
+static size_t __journal_remove_journal_head(struct buffer_head *bh)
 {
 	struct journal_head *jh = bh2jh(bh);
+	size_t b_size = READ_ONCE(bh->b_size);
 
 	J_ASSERT_JH(jh, jh->b_jcount >= 0);
 	J_ASSERT_JH(jh, jh->b_transaction == NULL);
@@ -2533,17 +2534,25 @@ static void __journal_remove_journal_hea
 	J_ASSERT_BH(bh, buffer_jbd(bh));
 	J_ASSERT_BH(bh, jh2bh(jh) == bh);
 	BUFFER_TRACE(bh, "remove journal_head");
+
+	/* Unlink before dropping the lock */
+	bh->b_private = NULL;
+	jh->b_bh = NULL;	/* debug, really */
+	clear_buffer_jbd(bh);
+
+	return b_size;
+}
+
+static void journal_release_journal_head(struct journal_head *jh, size_t b_size)
+{
 	if (jh->b_frozen_data) {
 		printk(KERN_WARNING "%s: freeing b_frozen_data\n", __func__);
-		jbd2_free(jh->b_frozen_data, bh->b_size);
+		jbd2_free(jh->b_frozen_data, b_size);
 	}
 	if (jh->b_committed_data) {
 		printk(KERN_WARNING "%s: freeing b_committed_data\n", __func__);
-		jbd2_free(jh->b_committed_data, bh->b_size);
+		jbd2_free(jh->b_committed_data, b_size);
 	}
-	bh->b_private = NULL;
-	jh->b_bh = NULL;	/* debug, really */
-	clear_buffer_jbd(bh);
 	journal_free_journal_head(jh);
 }
 
@@ -2559,11 +2568,14 @@ void jbd2_journal_put_journal_head(struc
 	J_ASSERT_JH(jh, jh->b_jcount > 0);
 	--jh->b_jcount;
 	if (!jh->b_jcount) {
-		__journal_remove_journal_head(bh);
+		size_t b_size = __journal_remove_journal_head(bh);
+
 		jbd_unlock_bh_journal_head(bh);
+		journal_release_journal_head(jh, b_size);
 		__brelse(bh);
-	} else
+	} else {
 		jbd_unlock_bh_journal_head(bh);
+	}
 }
 
 /*



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

* Re: [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state()
  2019-08-01  1:01 ` [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state() Thomas Gleixner
@ 2019-08-01  9:00   ` Jan Kara
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Kara @ 2019-08-01  9:00 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright,
	linux-ext4, Theodore Tso, Jan Kara, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

On Thu 01-08-19 03:01:30, Thomas Gleixner wrote:
> No users.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-ext4@vger.kernel.org
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Jan Kara <jack@suse.com>

Right.

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/jbd2.h |    5 -----
>  1 file changed, 5 deletions(-)
> 
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -347,11 +347,6 @@ static inline void jbd_lock_bh_state(str
>  	bit_spin_lock(BH_State, &bh->b_state);
>  }
>  
> -static inline int jbd_trylock_bh_state(struct buffer_head *bh)
> -{
> -	return bit_spin_trylock(BH_State, &bh->b_state);
> -}
> -
>  static inline int jbd_is_locked_bh_state(struct buffer_head *bh)
>  {
>  	return bit_spin_is_locked(BH_State, &bh->b_state);
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer()
  2019-08-01  1:01 ` [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer() Thomas Gleixner
@ 2019-08-01  9:04   ` Jan Kara
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Kara @ 2019-08-01  9:04 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright,
	linux-ext4, Theodore Tso, Jan Kara, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

On Thu 01-08-19 03:01:31, Thomas Gleixner wrote:
> journal_unmap_buffer() checks first whether the buffer head is a journal.
> If so it takes locks and then invokes jbd2_journal_grab_journal_head()
> followed by another check whether this is journal head buffer.
> 
> The double checking is pointless.
> 
> Replace the initial check with jbd2_journal_grab_journal_head() which
> alredy checks whether the buffer head is actually a journal.
> 
> Allows also early access to the journal head pointer for the upcoming
> conversion of state lock to a regular spinlock.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-ext4@vger.kernel.org
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Jan Kara <jack@suse.com>

Nice simplification. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> V2: New patch
> ---
>  fs/jbd2/transaction.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2196,7 +2196,8 @@ static int journal_unmap_buffer(journal_
>  	 * holding the page lock. --sct
>  	 */
>  
> -	if (!buffer_jbd(bh))
> +	jh = jbd2_journal_grab_journal_head(bh);
> +	if (!jh)
>  		goto zap_buffer_unlocked;
>  
>  	/* OK, we have data buffer in journaled mode */
> @@ -2204,10 +2205,6 @@ static int journal_unmap_buffer(journal_
>  	jbd_lock_bh_state(bh);
>  	spin_lock(&journal->j_list_lock);
>  
> -	jh = jbd2_journal_grab_journal_head(bh);
> -	if (!jh)
> -		goto zap_buffer_no_jh;
> -
>  	/*
>  	 * We cannot remove the buffer from checkpoint lists until the
>  	 * transaction adding inode to orphan list (let's call it T)
> @@ -2329,7 +2326,6 @@ static int journal_unmap_buffer(journal_
>  	 */
>  	jh->b_modified = 0;
>  	jbd2_journal_put_journal_head(jh);
> -zap_buffer_no_jh:
>  	spin_unlock(&journal->j_list_lock);
>  	jbd_unlock_bh_state(bh);
>  	write_unlock(&journal->j_state_lock);
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 7/7] fs/jbd2: Free journal head outside of locked region
  2019-08-01  1:01 ` [patch V2 7/7] fs/jbd2: Free journal head outside of locked region Thomas Gleixner
@ 2019-08-01  9:22   ` Jan Kara
  2019-08-01 18:08     ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Kara @ 2019-08-01  9:22 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	linux-ext4, Theodore Tso, Jan Kara, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, Mark Fasheh, Joseph Qi,
	Joel Becker

On Thu 01-08-19 03:01:33, Thomas Gleixner wrote:
> On PREEMPT_RT bit-spinlocks have the same semantics as on PREEMPT_RT=n,
> i.e. they disable preemption. That means functions which are not safe to be
> called in preempt disabled context on RT trigger a might_sleep() assert.
> 
> The journal head bit spinlock is mostly held for short code sequences with
> trivial RT safe functionality, except for one place:
> 
> jbd2_journal_put_journal_head() invokes __journal_remove_journal_head()
> with the journal head bit spinlock held. __journal_remove_journal_head()
> invokes kmem_cache_free() which must not be called with preemption disabled
> on RT.
> 
> Jan suggested to rework the removal function so the actual free happens
> outside the bit-spinlocked region.
> 
> Split it into two parts:
> 
>   - Do the sanity checks and the buffer head detach under the lock
> 
>   - Do the actual free after dropping the lock
> 
> There is error case handling in the free part which needs to dereference
> the b_size field of the now detached buffer head. Due to paranoia (caused
> by ignorance) the size is retrieved in the detach function and handed into
> the free function. Might be over-engineered, but better safe than sorry.
> 
> This makes the journal head bit-spinlock usage RT compliant and also avoids
> nested locking which is not covered by lockdep.
> 
> Suggested-by: Jan Kara <jack@suse.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-ext4@vger.kernel.org
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Jan Kara <jack@suse.com>

Looks mostly good. Just a small suggestion for simplification below:

> @@ -2559,11 +2568,14 @@ void jbd2_journal_put_journal_head(struc
>  	J_ASSERT_JH(jh, jh->b_jcount > 0);
>  	--jh->b_jcount;
>  	if (!jh->b_jcount) {
> -		__journal_remove_journal_head(bh);
> +		size_t b_size = __journal_remove_journal_head(bh);
> +
>  		jbd_unlock_bh_journal_head(bh);
> +		journal_release_journal_head(jh, b_size);
>  		__brelse(bh);

The bh is pinned until you call __brelse(bh) above and bh->b_size doesn't
change during the lifetime of the buffer. So there's no need of
fetching bh->b_size in __journal_remove_journal_head() and passing it back.
You can just:

		journal_release_journal_head(jh, bh->b_size);

> -	} else
> +	} else {
>  		jbd_unlock_bh_journal_head(bh);
> +	}
>  }
>  

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01  1:01 ` [patch V2 6/7] fs/jbd2: Make state lock a spinlock Thomas Gleixner
@ 2019-08-01 11:28   ` Peter Zijlstra
  2019-08-02 13:31     ` Jan Kara
  2019-08-01 17:57   ` Jan Kara
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Zijlstra @ 2019-08-01 11:28 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Ingo Molnar, Sebastian Siewior, Anna-Maria Gleixner,
	Steven Rostedt, Julia Cartwright, Jan Kara, Theodore Tso,
	Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4, Jan Kara,
	Matthew Wilcox, Alexander Viro, linux-fsdevel

On Thu, Aug 01, 2019 at 03:01:32AM +0200, Thomas Gleixner wrote:

> @@ -1931,7 +1932,7 @@ static void __jbd2_journal_temp_unlink_b
>  	transaction_t *transaction;
>  	struct buffer_head *bh = jh2bh(jh);
>  
> -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> +	assert_spin_locked(&jh->state_lock);
>  	transaction = jh->b_transaction;
>  	if (transaction)
>  		assert_spin_locked(&transaction->t_journal->j_list_lock);

> @@ -2415,7 +2416,7 @@ void __jbd2_journal_file_buffer(struct j
>  	int was_dirty = 0;
>  	struct buffer_head *bh = jh2bh(jh);
>  
> -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> +	assert_spin_locked(&jh->state_lock);
>  	assert_spin_locked(&transaction->t_journal->j_list_lock);
>  
>  	J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);

> @@ -2500,7 +2501,7 @@ void __jbd2_journal_refile_buffer(struct
>  	int was_dirty, jlist;
>  	struct buffer_head *bh = jh2bh(jh);
>  
> -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> +	assert_spin_locked(&jh->state_lock);
>  	if (jh->b_transaction)
>  		assert_spin_locked(&jh->b_transaction->t_journal->j_list_lock);
>  

Do those want to be:

  lockdep_assert_held(&jh->state_lock);

instead? The difference is of course that lockdep_assert_held() requires
the current context to hold the lock, where assert_*_locked() merely
checks _someone_ holds it.

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01  1:01 ` [patch V2 6/7] fs/jbd2: Make state lock a spinlock Thomas Gleixner
  2019-08-01 11:28   ` Peter Zijlstra
@ 2019-08-01 17:57   ` Jan Kara
  2019-08-01 18:12     ` Thomas Gleixner
  1 sibling, 1 reply; 27+ messages in thread
From: Jan Kara @ 2019-08-01 17:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Jan Kara, Matthew Wilcox, Alexander Viro, linux-fsdevel

On Thu 01-08-19 03:01:32, Thomas Gleixner wrote:
> Bit-spinlocks are problematic on PREEMPT_RT if functions which might sleep
> on RT, e.g. spin_lock(), alloc/free(), are invoked inside the lock held
> region because bit spinlocks disable preemption even on RT.
> 
> A first attempt was to replace state lock with a spinlock placed in struct
> buffer_head and make the locking conditional on PREEMPT_RT and
> DEBUG_BIT_SPINLOCKS.
> 
> Jan pointed out that there is a 4 byte hole in struct journal_head where a
> regular spinlock fits in and he would not object to convert the state lock
> to a spinlock unconditionally.
> 
> Aside of solving the RT problem, this also gains lockdep coverage for the
> journal head state lock (bit-spinlocks are not covered by lockdep as it's
> hard to fit a lockdep map into a single bit).
> 
> The trivial change would have been to convert the jbd_*lock_bh_state()
> inlines, but that comes with the downside that these functions take a
> buffer head pointer which needs to be converted to a journal head pointer
> which adds another level of indirection.
> 
> As almost all functions which use this lock have a journal head pointer
> readily available, it makes more sense to remove the lock helper inlines
> and write out spin_*lock() at all call sites.
> 
> Fixup all locking comments as well.
> 
> Suggested-by: Jan Kara <jack@suse.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Mark Fasheh <mark@fasheh.com>
> Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Jan Kara <jack@suse.com>
> Cc: linux-ext4@vger.kernel.org

Just a heads up that I didn't miss this patch. Just it has some bugs and I
figured that rather than explaining to you subtleties of jh lifetime it is
easier to fix up the problems myself since you're probably not keen on
becoming jbd2 developer ;)... which was more complex than I thought so I'm
not completely done yet. Hopefuly tomorrow.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 7/7] fs/jbd2: Free journal head outside of locked region
  2019-08-01  9:22   ` Jan Kara
@ 2019-08-01 18:08     ` Thomas Gleixner
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01 18:08 UTC (permalink / raw)
  To: Jan Kara
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	linux-ext4, Theodore Tso, Matthew Wilcox, Alexander Viro,
	linux-fsdevel, Mark Fasheh, Joseph Qi, Joel Becker

On Thu, 1 Aug 2019, Jan Kara wrote:
> On Thu 01-08-19 03:01:33, Thomas Gleixner wrote:
> > On PREEMPT_RT bit-spinlocks have the same semantics as on PREEMPT_RT=n,
> > i.e. they disable preemption. That means functions which are not safe to be
> > called in preempt disabled context on RT trigger a might_sleep() assert.
> Looks mostly good. Just a small suggestion for simplification below:
> 
> > @@ -2559,11 +2568,14 @@ void jbd2_journal_put_journal_head(struc
> >  	J_ASSERT_JH(jh, jh->b_jcount > 0);
> >  	--jh->b_jcount;
> >  	if (!jh->b_jcount) {
> > -		__journal_remove_journal_head(bh);
> > +		size_t b_size = __journal_remove_journal_head(bh);
> > +
> >  		jbd_unlock_bh_journal_head(bh);
> > +		journal_release_journal_head(jh, b_size);
> >  		__brelse(bh);
> 
> The bh is pinned until you call __brelse(bh) above and bh->b_size doesn't
> change during the lifetime of the buffer. So there's no need of
> fetching bh->b_size in __journal_remove_journal_head() and passing it back.
> You can just:
> 
> 		journal_release_journal_head(jh, bh->b_size);

Ah. Nice. I assumed that this would be possible, but then my ignorance
induced paranoia won.

Thanks,

	tglx

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01 17:57   ` Jan Kara
@ 2019-08-01 18:12     ` Thomas Gleixner
  2019-08-02 13:37       ` Jan Kara
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-01 18:12 UTC (permalink / raw)
  To: Jan Kara
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Matthew Wilcox, Alexander Viro, linux-fsdevel

On Thu, 1 Aug 2019, Jan Kara wrote:
> On Thu 01-08-19 03:01:32, Thomas Gleixner wrote:
> > As almost all functions which use this lock have a journal head pointer
> > readily available, it makes more sense to remove the lock helper inlines
> > and write out spin_*lock() at all call sites.
> > 
> 
> Just a heads up that I didn't miss this patch. Just it has some bugs and I
> figured that rather than explaining to you subtleties of jh lifetime it is
> easier to fix up the problems myself since you're probably not keen on
> becoming jbd2 developer ;)... which was more complex than I thought so I'm
> not completely done yet. Hopefuly tomorrow.

I'm curious where I was too naive :)

Thanks for having a look!

       tglx

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
                   ` (6 preceding siblings ...)
  2019-08-01  1:01 ` [patch V2 7/7] fs/jbd2: Free journal head outside of locked region Thomas Gleixner
@ 2019-08-02  7:56 ` Christoph Hellwig
  2019-08-02  9:07   ` Thomas Gleixner
  7 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2019-08-02  7:56 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

Hi Thomas,

did you look into killing bіt spinlocks as a public API instead?

The main users seems to be buffer heads, which are so bloated that
an extra spinlock doesn't really matter anyway.

The list_bl and rhashtable uses kinda make sense to be, but they are
pretty nicely abstracted away anyway.  The remaining users look
pretty questionable to start with.

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-02  7:56 ` [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Christoph Hellwig
@ 2019-08-02  9:07   ` Thomas Gleixner
  2019-08-06  6:11     ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-02  9:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

Christoph,

On Fri, 2 Aug 2019, Christoph Hellwig wrote:

> did you look into killing bіt spinlocks as a public API instead?

Last time I did, there was resistance :)

But I'm happy to try again.

> The main users seems to be buffer heads, which are so bloated that
> an extra spinlock doesn't really matter anyway.
>
> The list_bl and rhashtable uses kinda make sense to be, but they are
> pretty nicely abstracted away anyway.  The remaining users look
> pretty questionable to start with.

What about the page lock?

  mm/slub.c:      bit_spin_lock(PG_locked, &page->flags);

Thanks,

	tglx

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01 11:28   ` Peter Zijlstra
@ 2019-08-02 13:31     ` Jan Kara
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Kara @ 2019-08-02 13:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thomas Gleixner, LKML, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Jan Kara, Matthew Wilcox, Alexander Viro, linux-fsdevel

On Thu 01-08-19 13:28:49, Peter Zijlstra wrote:
> On Thu, Aug 01, 2019 at 03:01:32AM +0200, Thomas Gleixner wrote:
> 
> > @@ -1931,7 +1932,7 @@ static void __jbd2_journal_temp_unlink_b
> >  	transaction_t *transaction;
> >  	struct buffer_head *bh = jh2bh(jh);
> >  
> > -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> > +	assert_spin_locked(&jh->state_lock);
> >  	transaction = jh->b_transaction;
> >  	if (transaction)
> >  		assert_spin_locked(&transaction->t_journal->j_list_lock);
> 
> > @@ -2415,7 +2416,7 @@ void __jbd2_journal_file_buffer(struct j
> >  	int was_dirty = 0;
> >  	struct buffer_head *bh = jh2bh(jh);
> >  
> > -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> > +	assert_spin_locked(&jh->state_lock);
> >  	assert_spin_locked(&transaction->t_journal->j_list_lock);
> >  
> >  	J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);
> 
> > @@ -2500,7 +2501,7 @@ void __jbd2_journal_refile_buffer(struct
> >  	int was_dirty, jlist;
> >  	struct buffer_head *bh = jh2bh(jh);
> >  
> > -	J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh));
> > +	assert_spin_locked(&jh->state_lock);
> >  	if (jh->b_transaction)
> >  		assert_spin_locked(&jh->b_transaction->t_journal->j_list_lock);
> >  
> 
> Do those want to be:
> 
>   lockdep_assert_held(&jh->state_lock);
> 
> instead? The difference is of course that lockdep_assert_held() requires
> the current context to hold the lock, where assert_*_locked() merely
> checks _someone_ holds it.

Yeah, jbd2 doesn't play any weird locking tricks so lockdep_assert_held()
is fine. I'll replace those when I'm updating the patch.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-01 18:12     ` Thomas Gleixner
@ 2019-08-02 13:37       ` Jan Kara
  2019-08-02 15:29         ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Kara @ 2019-08-02 13:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Jan Kara, LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Matthew Wilcox, Alexander Viro, linux-fsdevel

On Thu 01-08-19 20:12:11, Thomas Gleixner wrote:
> On Thu, 1 Aug 2019, Jan Kara wrote:
> > On Thu 01-08-19 03:01:32, Thomas Gleixner wrote:
> > > As almost all functions which use this lock have a journal head pointer
> > > readily available, it makes more sense to remove the lock helper inlines
> > > and write out spin_*lock() at all call sites.
> > > 
> > 
> > Just a heads up that I didn't miss this patch. Just it has some bugs and I
> > figured that rather than explaining to you subtleties of jh lifetime it is
> > easier to fix up the problems myself since you're probably not keen on
> > becoming jbd2 developer ;)... which was more complex than I thought so I'm
> > not completely done yet. Hopefuly tomorrow.
> 
> I'm curious where I was too naive :)

Well, the most obvious where places where the result ended up being like

	jbd2_journal_put_journal_head(jh);
	spin_unlock(&jh->state_lock);

That's possible use-after-free.

But there were also other more subtle places where
jbd2_journal_put_journal_head() was not directly visible as it was hidden
inside journal list handling functions such as __jbd2_journal_refile_buffer()
or so. And these needed some more work.

Anyway, I'm now done fixing up the patch, doing some xfstests runs to verify
things didn't break in any obvious way...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [patch V2 6/7] fs/jbd2: Make state lock a spinlock
  2019-08-02 13:37       ` Jan Kara
@ 2019-08-02 15:29         ` Thomas Gleixner
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-02 15:29 UTC (permalink / raw)
  To: Jan Kara
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Mark Fasheh, Joseph Qi, Joel Becker, linux-ext4,
	Matthew Wilcox, Alexander Viro, linux-fsdevel

On Fri, 2 Aug 2019, Jan Kara wrote:

> On Thu 01-08-19 20:12:11, Thomas Gleixner wrote:
> > On Thu, 1 Aug 2019, Jan Kara wrote:
> > > On Thu 01-08-19 03:01:32, Thomas Gleixner wrote:
> > > > As almost all functions which use this lock have a journal head pointer
> > > > readily available, it makes more sense to remove the lock helper inlines
> > > > and write out spin_*lock() at all call sites.
> > > > 
> > > 
> > > Just a heads up that I didn't miss this patch. Just it has some bugs and I
> > > figured that rather than explaining to you subtleties of jh lifetime it is
> > > easier to fix up the problems myself since you're probably not keen on
> > > becoming jbd2 developer ;)... which was more complex than I thought so I'm
> > > not completely done yet. Hopefuly tomorrow.
> > 
> > I'm curious where I was too naive :)
> 
> Well, the most obvious where places where the result ended up being like
> 
> 	jbd2_journal_put_journal_head(jh);
> 	spin_unlock(&jh->state_lock);
> 
> That's possible use-after-free.

Duh yes.

> But there were also other more subtle places where
> jbd2_journal_put_journal_head() was not directly visible as it was hidden
> inside journal list handling functions such as __jbd2_journal_refile_buffer()
> or so. And these needed some more work.
> 
> Anyway, I'm now done fixing up the patch, doing some xfstests runs to verify
> things didn't break in any obvious way...

Very appreciated.

Thanks,

	tglx

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-02  9:07   ` Thomas Gleixner
@ 2019-08-06  6:11     ` Christoph Hellwig
  2019-08-08  7:02       ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2019-08-06  6:11 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Christoph Hellwig, LKML, Peter Zijlstra, Ingo Molnar,
	Sebastian Siewior, Anna-Maria Gleixner, Steven Rostedt,
	Julia Cartwright, Jan Kara, Theodore Tso, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, linux-ext4, Jan Kara, Mark Fasheh,
	Joseph Qi, Joel Becker

On Fri, Aug 02, 2019 at 11:07:53AM +0200, Thomas Gleixner wrote:
> Last time I did, there was resistance :)

Do you have a pointer?  Note that in the buffer head case maybe
a hash lock based on the page address is even better, as we only
ever use the lock in the first buffer head of a page anyway..

> What about the page lock?
> 
>   mm/slub.c:      bit_spin_lock(PG_locked, &page->flags);

One caller ouf of a gazillion that spins on the page lock instead of
sleepign on it like everyone else.  That should not have passed your
smell test to start with :)

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-06  6:11     ` Christoph Hellwig
@ 2019-08-08  7:02       ` Thomas Gleixner
  2019-08-08  7:28         ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-08  7:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

On Mon, 5 Aug 2019, Christoph Hellwig wrote:
> On Fri, Aug 02, 2019 at 11:07:53AM +0200, Thomas Gleixner wrote:
> > Last time I did, there was resistance :)
> 
> Do you have a pointer?  Note that in the buffer head case maybe
> a hash lock based on the page address is even better, as we only
> ever use the lock in the first buffer head of a page anyway..

I need to search my archives, but I'm on a spotty and slow connection right
now. Will do so when back home.
 
> > What about the page lock?
> > 
> >   mm/slub.c:      bit_spin_lock(PG_locked, &page->flags);
> 
> One caller ouf of a gazillion that spins on the page lock instead of
> sleepign on it like everyone else.  That should not have passed your
> smell test to start with :)

I surely stared at it, but that cannot sleep. It's in the middle of a
preempt and interrupt disabled region and used on architectures which do
not support CMPXCHG_DOUBLE and ALIGNED_STRUCT_PAGE ...

Thanks,

	tglx




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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-08  7:02       ` Thomas Gleixner
@ 2019-08-08  7:28         ` Christoph Hellwig
  2019-08-08  7:54           ` Thomas Gleixner
  0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2019-08-08  7:28 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Christoph Hellwig, LKML, Peter Zijlstra, Ingo Molnar,
	Sebastian Siewior, Anna-Maria Gleixner, Steven Rostedt,
	Julia Cartwright, Jan Kara, Theodore Tso, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, linux-ext4, Jan Kara, Mark Fasheh,
	Joseph Qi, Joel Becker

On Thu, Aug 08, 2019 at 09:02:47AM +0200, Thomas Gleixner wrote:
> > >   mm/slub.c:      bit_spin_lock(PG_locked, &page->flags);
> > 
> > One caller ouf of a gazillion that spins on the page lock instead of
> > sleepign on it like everyone else.  That should not have passed your
> > smell test to start with :)
> 
> I surely stared at it, but that cannot sleep. It's in the middle of a
> preempt and interrupt disabled region and used on architectures which do
> not support CMPXCHG_DOUBLE and ALIGNED_STRUCT_PAGE ...

I know.  But the problem here is that normally PG_locked is used together 
with wait_on_page_bit_*, but this one instances uses the bit spinlock
helpers.  This is the equivalent of calling spin_lock on a struct mutex
rather than having a mutex_lock_spin helper for this case.  Does SLUB
work on -rt at all?

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-08  7:28         ` Christoph Hellwig
@ 2019-08-08  7:54           ` Thomas Gleixner
  2019-08-10  8:18             ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2019-08-08  7:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Sebastian Siewior,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

On Thu, 8 Aug 2019, Christoph Hellwig wrote:
> On Thu, Aug 08, 2019 at 09:02:47AM +0200, Thomas Gleixner wrote:
> > > >   mm/slub.c:      bit_spin_lock(PG_locked, &page->flags);
> > > 
> > > One caller ouf of a gazillion that spins on the page lock instead of
> > > sleepign on it like everyone else.  That should not have passed your
> > > smell test to start with :)
> > 
> > I surely stared at it, but that cannot sleep. It's in the middle of a
> > preempt and interrupt disabled region and used on architectures which do
> > not support CMPXCHG_DOUBLE and ALIGNED_STRUCT_PAGE ...
> 
> I know.  But the problem here is that normally PG_locked is used together 
> with wait_on_page_bit_*, but this one instances uses the bit spinlock
> helpers.  This is the equivalent of calling spin_lock on a struct mutex
> rather than having a mutex_lock_spin helper for this case.

Yes, I know :(

> Does SLUB work on -rt at all?

It's the only allocator we support with a few tweaks :)

Thanks,

	tglx

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-08  7:54           ` Thomas Gleixner
@ 2019-08-10  8:18             ` Christoph Hellwig
  2019-08-11  1:22               ` Matthew Wilcox
  2019-08-20 17:16               ` Sebastian Siewior
  0 siblings, 2 replies; 27+ messages in thread
From: Christoph Hellwig @ 2019-08-10  8:18 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Christoph Hellwig, LKML, Peter Zijlstra, Ingo Molnar,
	Sebastian Siewior, Anna-Maria Gleixner, Steven Rostedt,
	Julia Cartwright, Jan Kara, Theodore Tso, Matthew Wilcox,
	Alexander Viro, linux-fsdevel, linux-ext4, Jan Kara, Mark Fasheh,
	Joseph Qi, Joel Becker

On Thu, Aug 08, 2019 at 09:54:03AM +0200, Thomas Gleixner wrote:
> > I know.  But the problem here is that normally PG_locked is used together 
> > with wait_on_page_bit_*, but this one instances uses the bit spinlock
> > helpers.  This is the equivalent of calling spin_lock on a struct mutex
> > rather than having a mutex_lock_spin helper for this case.
> 
> Yes, I know :(

But this means we should exclude slub from the bit_spin_lock removal.
It really should use it's own version of it anyhow insted of pretending
that the page lock is a bit spinlock.

> 
> > Does SLUB work on -rt at all?
> 
> It's the only allocator we support with a few tweaks :)

What do you do about this particular piece of code there?

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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-10  8:18             ` Christoph Hellwig
@ 2019-08-11  1:22               ` Matthew Wilcox
  2019-08-20 17:16               ` Sebastian Siewior
  1 sibling, 0 replies; 27+ messages in thread
From: Matthew Wilcox @ 2019-08-11  1:22 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Thomas Gleixner, LKML, Peter Zijlstra, Ingo Molnar,
	Sebastian Siewior, Anna-Maria Gleixner, Steven Rostedt,
	Julia Cartwright, Jan Kara, Theodore Tso, Alexander Viro,
	linux-fsdevel, linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi,
	Joel Becker

On Sat, Aug 10, 2019 at 01:18:34AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 08, 2019 at 09:54:03AM +0200, Thomas Gleixner wrote:
> > > I know.  But the problem here is that normally PG_locked is used together 
> > > with wait_on_page_bit_*, but this one instances uses the bit spinlock
> > > helpers.  This is the equivalent of calling spin_lock on a struct mutex
> > > rather than having a mutex_lock_spin helper for this case.
> > 
> > Yes, I know :(
> 
> But this means we should exclude slub from the bit_spin_lock removal.
> It really should use it's own version of it anyhow insted of pretending
> that the page lock is a bit spinlock.

But PG_locked isn't used as a mutex _when the page is allocated by slab_.
Yes, every other user uses PG_locked as a mutex, but I don't see why that
should constrain slub's usage of PG_locked.


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

* Re: [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging
  2019-08-10  8:18             ` Christoph Hellwig
  2019-08-11  1:22               ` Matthew Wilcox
@ 2019-08-20 17:16               ` Sebastian Siewior
  1 sibling, 0 replies; 27+ messages in thread
From: Sebastian Siewior @ 2019-08-20 17:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Thomas Gleixner, LKML, Peter Zijlstra, Ingo Molnar,
	Anna-Maria Gleixner, Steven Rostedt, Julia Cartwright, Jan Kara,
	Theodore Tso, Matthew Wilcox, Alexander Viro, linux-fsdevel,
	linux-ext4, Jan Kara, Mark Fasheh, Joseph Qi, Joel Becker

On 2019-08-10 01:18:34 [-0700], Christoph Hellwig wrote:
> > > Does SLUB work on -rt at all?
> > 
> > It's the only allocator we support with a few tweaks :)
> 
> What do you do about this particular piece of code there?

This part remains untouched. This "lock" is acquired within ->list_lock
which is a raw_spinlock_t and disables preemption/interrupts on -RT.

Sebastian

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

end of thread, other threads:[~2019-08-20 17:17 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01  1:01 [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Thomas Gleixner
2019-08-01  1:01 ` [patch V2 1/7] locking/lockdep: Add Kconfig option for bit spinlocks Thomas Gleixner
2019-08-01  1:01 ` [patch V2 2/7] fs/buffer: Move BH_Uptodate_Lock locking into wrapper functions Thomas Gleixner
2019-08-01  1:01 ` [patch V2 3/7] fs/buffer: Substitute BH_Uptodate_Lock for RT and bit spinlock debugging Thomas Gleixner
2019-08-01  1:01 ` [patch V2 4/7] fs/jbd2: Remove jbd_trylock_bh_state() Thomas Gleixner
2019-08-01  9:00   ` Jan Kara
2019-08-01  1:01 ` [patch V2 5/7] fs/jbd2: Simplify journal_unmap_buffer() Thomas Gleixner
2019-08-01  9:04   ` Jan Kara
2019-08-01  1:01 ` [patch V2 6/7] fs/jbd2: Make state lock a spinlock Thomas Gleixner
2019-08-01 11:28   ` Peter Zijlstra
2019-08-02 13:31     ` Jan Kara
2019-08-01 17:57   ` Jan Kara
2019-08-01 18:12     ` Thomas Gleixner
2019-08-02 13:37       ` Jan Kara
2019-08-02 15:29         ` Thomas Gleixner
2019-08-01  1:01 ` [patch V2 7/7] fs/jbd2: Free journal head outside of locked region Thomas Gleixner
2019-08-01  9:22   ` Jan Kara
2019-08-01 18:08     ` Thomas Gleixner
2019-08-02  7:56 ` [patch V2 0/7] fs: Substitute bit-spinlocks for PREEMPT_RT and debugging Christoph Hellwig
2019-08-02  9:07   ` Thomas Gleixner
2019-08-06  6:11     ` Christoph Hellwig
2019-08-08  7:02       ` Thomas Gleixner
2019-08-08  7:28         ` Christoph Hellwig
2019-08-08  7:54           ` Thomas Gleixner
2019-08-10  8:18             ` Christoph Hellwig
2019-08-11  1:22               ` Matthew Wilcox
2019-08-20 17:16               ` Sebastian Siewior

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).