[13/22] jbd2: Drop pointless check from jbd2_journal_stop()
diff mbox series

Message ID 20191003220613.10791-13-jack@suse.cz
State Superseded
Headers show
Series
  • ext4: Fix transaction overflow due to revoke descriptors
Related show

Commit Message

Jan Kara Oct. 3, 2019, 10:05 p.m. UTC
If a transaction is larger than journal->j_max_transaction_buffers, that
is a bug and not a trigger for transaction commit. Also the very next
attempt to start new handle will start transaction commit anyway. So
just remove the pointless check. Arguably, we could start transaction
commit whenever the transaction size is *close* to
journal->j_max_transaction_buffers. This has a potential to reduce
latency of the next jbd2_journal_start() at the cost of somewhat smaller
transactions. However for this to have any effect, it would mean that
there isn't someone already waiting in jbd2_journal_start() which means
metadata load for the fs is pretty light anyway so probably this
optimization is not worth it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jbd2/transaction.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Theodore Y. Ts'o Oct. 21, 2019, 5:30 p.m. UTC | #1
On Fri, Oct 04, 2019 at 12:05:59AM +0200, Jan Kara wrote:
> If a transaction is larger than journal->j_max_transaction_buffers, that
> is a bug and not a trigger for transaction commit. Also the very next
> attempt to start new handle will start transaction commit anyway. So
> just remove the pointless check. Arguably, we could start transaction
> commit whenever the transaction size is *close* to
> journal->j_max_transaction_buffers. This has a potential to reduce
> latency of the next jbd2_journal_start() at the cost of somewhat smaller
> transactions. However for this to have any effect, it would mean that
> there isn't someone already waiting in jbd2_journal_start() which means
> metadata load for the fs is pretty light anyway so probably this
> optimization is not worth it.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks good; feel free to add:

Reviewed-by: Theodore Ts'o <tytso@mit.edu>

Patch
diff mbox series

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 6f560713f7f0..a160c3f665f9 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1803,13 +1803,10 @@  int jbd2_journal_stop(handle_t *handle)
 
 	/*
 	 * If the handle is marked SYNC, we need to set another commit
-	 * going!  We also want to force a commit if the current
-	 * transaction is occupying too much of the log, or if the
-	 * transaction is too old now.
+	 * going!  We also want to force a commit if the transaction is too
+	 * old now.
 	 */
 	if (handle->h_sync ||
-	    (atomic_read(&transaction->t_outstanding_credits) >
-	     journal->j_max_transaction_buffers) ||
 	    time_after_eq(jiffies, transaction->t_expires)) {
 		/* Do this even for aborted journals: an abort still
 		 * completes the commit thread, it just doesn't write