From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Jan Kara <jack@suse.cz>
Cc: linux-ext4@vger.kernel.org, Adrian Hunter <adrian.hunter@intel.com>
Subject: Re: [PATCH] jbd2: Avoid long hold times of j_state_lock while committing a transaction
Date: Tue, 6 Nov 2018 11:47:59 -0500 [thread overview]
Message-ID: <20181106164759.GE6416@thunk.org> (raw)
In-Reply-To: <20181106102230.GC25414@quack2.suse.cz>
On Tue, Nov 06, 2018 at 11:22:30AM +0100, Jan Kara wrote:
>> So the buffer is on BJ_Shadow list while the assertion in
> jbd2_journal_dirty_metadata() expects it to be in BJ_Metadata list. This is
> really weird as we have also checked that jh->b_transaction ==
> handle->h_transaction so the transaction couldn't have passed to commit
> phase... Oh, I see, the code in start_this_handle() got racy with the
> removal of j_state_lock protection from journal_commit_transaction() so now
> transaction can start even though there are handles outstanding! I'll think
> about the best solution for this. Thanks for report!
Thanks for the analysis! I finished the bisection last night and it
was too late for me to dive into how this was going on. I should have
realized this before I had suggested the approach in the patch.
The original complaint which Andrian made was that the long hold times
of j_state_lock at the beginning of the commit. What he didn't
mention was what the other "high priority tasks" were blocked on, but
they were almost certainly start_this_handle. And that's fundamental;
when we are trying to at the beginning of the commit process is
waiting for the outstanding handles to close; and so we can't let new
handles start.
What we can do is to try to decrease the handle hold times. This is
why we track the handle type and we have the jbd2_handle_stats
tracepoint. If we can find that handles of a particular type and a
particular line number are the ones which are taking more time than
other handles, we can try to make them run faster; for example, by
pre-reading blocks aren't in the buffer cache before starting the
handle.
The other thing which we can probably do is to for truncate handles,
if we notice that current_transaction->t_state == T_LOCKED, we can
suspend the truncation activity and close the handle, and then resume
it (which will block until a new transaction is ready to be started).
- Ted
next prev parent reply other threads:[~2018-11-07 2:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-16 9:42 [PATCH] jbd2: Avoid long hold times of j_state_lock while committing a transaction Jan Kara
2018-10-30 14:44 ` Theodore Y. Ts'o
2018-11-06 5:20 ` Theodore Y. Ts'o
2018-11-06 10:22 ` Jan Kara
2018-11-06 16:47 ` Theodore Y. Ts'o [this message]
2018-11-06 17:14 ` Hunter, Adrian
2018-11-06 17:21 ` Jan Kara
2018-11-08 12:30 ` 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=20181106164759.GE6416@thunk.org \
--to=tytso@mit.edu \
--cc=adrian.hunter@intel.com \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
/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).