From patchwork Thu Nov 8 12:29:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 994845 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-ext4-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 42rMzw1dJ2z9sC7 for ; Thu, 8 Nov 2018 23:29:48 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726599AbeKHWFA (ORCPT ); Thu, 8 Nov 2018 17:05:00 -0500 Received: from mx2.suse.de ([195.135.220.15]:55706 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726609AbeKHWE7 (ORCPT ); Thu, 8 Nov 2018 17:04:59 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 03A46AF8D; Thu, 8 Nov 2018 12:29:42 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id C11401E09A5; Thu, 8 Nov 2018 13:29:41 +0100 (CET) From: Jan Kara To: Ted Tso Cc: , "Hunter, Adrian" , Jan Kara Subject: [PATCH v2] jbd2: Avoid long hold times of j_state_lock while committing a transaction Date: Thu, 8 Nov 2018 13:29:38 +0100 Message-Id: <20181108122938.10688-1-jack@suse.cz> X-Mailer: git-send-email 2.16.4 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org We can hold j_state_lock for writing at the beginning of jbd2_journal_commit_transaction() for a rather long time (reportedly for 30 ms) due cleaning revoke bits of all revoked buffers under it. The handling of revoke tables as well as cleaning of t_reserved_list, and checkpoint lists does not need j_state_lock for anything. It is only needed to prevent new handles from joining the transaction. Generally T_LOCKED transaction state prevents new handles from joining the transaction - except for reserved handles which have to allowed to join while we wait for other handles to complete. To prevent reserved handles from joining the transaction while cleaning up lists, add new transaction state T_SWITCH and watch for it when starting reserved handles. With this we can just drop the lock for operations that don't need it. Reported-and-tested-by: Adrian Hunter Suggested-by: "Theodore Y. Ts'o" Signed-off-by: Jan Kara --- fs/jbd2/commit.c | 3 +++ fs/jbd2/transaction.c | 43 ++++++++++++++++++++++++++++++++++++++----- include/linux/jbd2.h | 1 + 3 files changed, 42 insertions(+), 5 deletions(-) Changes since v1: - fixed handling of reserved handles - rebased on top of 4.20-rc1 - tested dioread-nolock xfstest run diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c index 150cc030b4d7..2eb55c3361a8 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -439,6 +439,8 @@ void jbd2_journal_commit_transaction(journal_t *journal) finish_wait(&journal->j_wait_updates, &wait); } spin_unlock(&commit_transaction->t_handle_lock); + commit_transaction->t_state = T_SWITCH; + write_unlock(&journal->j_state_lock); J_ASSERT (atomic_read(&commit_transaction->t_outstanding_credits) <= journal->j_max_transaction_buffers); @@ -505,6 +507,7 @@ void jbd2_journal_commit_transaction(journal_t *journal) atomic_sub(atomic_read(&journal->j_reserved_credits), &commit_transaction->t_outstanding_credits); + write_lock(&journal->j_state_lock); trace_jbd2_commit_flushing(journal, commit_transaction); stats.run.rs_flushing = jiffies; stats.run.rs_locked = jbd2_time_diff(stats.run.rs_locked, diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index c0b66a7a795b..116d8251fbff 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -138,9 +138,9 @@ static inline void update_t_max_wait(transaction_t *transaction, } /* - * Wait until running transaction passes T_LOCKED state. Also starts the commit - * if needed. The function expects running transaction to exist and releases - * j_state_lock. + * Wait until running transaction passes to T_FLUSH state and new transaction + * can thus be started. Also starts the commit if needed. The function expects + * running transaction to exist and releases j_state_lock. */ static void wait_transaction_locked(journal_t *journal) __releases(journal->j_state_lock) @@ -160,6 +160,32 @@ static void wait_transaction_locked(journal_t *journal) finish_wait(&journal->j_wait_transaction_locked, &wait); } +/* + * Wait until running transaction transitions from T_SWITCH to T_FLUSH + * state and new transaction can thus be started. The function releases + * j_state_lock. + */ +static void wait_transaction_switching(journal_t *journal) + __releases(journal->j_state_lock) +{ + DEFINE_WAIT(wait); + + if (WARN_ON(!journal->j_running_transaction || + journal->j_running_transaction->t_state != T_SWITCH)) + return; + prepare_to_wait(&journal->j_wait_transaction_locked, &wait, + TASK_UNINTERRUPTIBLE); + read_unlock(&journal->j_state_lock); + /* + * We don't call jbd2_might_wait_for_commit() here as there's no + * waiting for outstanding handles happening anymore in T_SWITCH state + * and handling of reserved handles actually relies on that for + * correctness. + */ + schedule(); + finish_wait(&journal->j_wait_transaction_locked, &wait); +} + static void sub_reserved_credits(journal_t *journal, int blocks) { atomic_sub(blocks, &journal->j_reserved_credits); @@ -183,7 +209,8 @@ static int add_transaction_credits(journal_t *journal, int blocks, * If the current transaction is locked down for commit, wait * for the lock to be released. */ - if (t->t_state == T_LOCKED) { + if (t->t_state != T_RUNNING) { + WARN_ON_ONCE(t->t_state >= T_FLUSH); wait_transaction_locked(journal); return 1; } @@ -360,8 +387,14 @@ static int start_this_handle(journal_t *journal, handle_t *handle, /* * We have handle reserved so we are allowed to join T_LOCKED * transaction and we don't have to check for transaction size - * and journal space. + * and journal space. But we still have to wait while running + * transaction is being switched to a committing one as it + * won't wait for any handles anymore. */ + if (transaction->t_state == T_SWITCH) { + wait_transaction_switching(journal); + goto repeat; + } sub_reserved_credits(journal, blocks); handle->h_reserved = 0; } diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h index b708e5169d1d..118d00a64184 100644 --- a/include/linux/jbd2.h +++ b/include/linux/jbd2.h @@ -575,6 +575,7 @@ struct transaction_s enum { T_RUNNING, T_LOCKED, + T_SWITCH, T_FLUSH, T_COMMIT, T_COMMIT_DFLUSH,