From: Jan Kara <jack@suse.cz>
To: Ted Tso <tytso@mit.edu>
Cc: <linux-ext4@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Anna-Maria Gleixner <anna-maria@linutronix.de>,
Julia Cartwright <julia@ni.com>, Jan Kara <jack@suse.com>,
Jan Kara <jack@suse.cz>
Subject: [PATCH 7/7] jbd2: Free journal head outside of locked region
Date: Fri, 9 Aug 2019 14:42:33 +0200 [thread overview]
Message-ID: <20190809124233.13277-8-jack@suse.cz> (raw)
In-Reply-To: <20190809124233.13277-1-jack@suse.cz>
From: Thomas Gleixner <tglx@linutronix.de>
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>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/journal.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index eee350fef339..c79e6de2fcfc 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2533,17 +2533,23 @@ static void __journal_remove_journal_head(struct buffer_head *bh)
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);
+}
+
+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);
}
@@ -2561,9 +2567,11 @@ void jbd2_journal_put_journal_head(struct journal_head *jh)
if (!jh->b_jcount) {
__journal_remove_journal_head(bh);
jbd_unlock_bh_journal_head(bh);
+ journal_release_journal_head(jh, bh->b_size);
__brelse(bh);
- } else
+ } else {
jbd_unlock_bh_journal_head(bh);
+ }
}
/*
--
2.16.4
next prev parent reply other threads:[~2019-08-09 12:42 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-09 12:42 [PATCH 0/7 v2] jbd2: Bit spinlock conversions Jan Kara
2019-08-09 12:42 ` [PATCH 1/7] jbd2: Simplify journal_unmap_buffer() Jan Kara
2019-08-09 12:42 ` [PATCH 2/7] jbd2: Remove jbd_trylock_bh_state() Jan Kara
2019-08-09 12:42 ` [PATCH 3/7] jbd2: Move dropping of jh reference out of un/re-filing functions Jan Kara
2019-08-09 12:42 ` [PATCH 4/7] jbd2: Drop unnecessary branch from jbd2_journal_forget() Jan Kara
2019-08-09 12:42 ` [PATCH 5/7] jbd2: Don't call __bforget() unnecessarily Jan Kara
2019-10-28 15:28 ` Theodore Y. Ts'o
2019-10-28 16:01 ` Theodore Y. Ts'o
2019-10-30 11:49 ` Jan Kara
2019-08-09 12:42 ` [PATCH 6/7] jbd2: Make state lock a spinlock Jan Kara
2019-08-09 12:42 ` Jan Kara [this message]
2019-10-11 12:31 ` [PATCH 0/7 v2] jbd2: Bit spinlock conversions Sebastian Andrzej Siewior
2019-11-03 19:01 ` Theodore Y. Ts'o
-- strict thread matches above, loose matches on Subject: below --
2019-08-02 15:13 [PATCH 0/7] " Jan Kara
2019-08-02 15:13 ` [PATCH 7/7] jbd2: Free journal head outside of locked region Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190809124233.13277-8-jack@suse.cz \
--to=jack@suse.cz \
--cc=anna-maria@linutronix.de \
--cc=jack@suse.com \
--cc=julia@ni.com \
--cc=linux-ext4@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).