[PATCH] JBD: log space management optimization

[PATCH] JBD: log space management optimization

[Alex Tomas]

Good day,

during truncate ext3 calls journal_forget() for freed blocks, but
before these blocks go to the transaction and jbd reserves space
in log for them (->t_outstanding_credits). also, journal_forget()
removes these blocks from the transaction, but doesn't correct
log space reservation. for example, removal of 500MB file reserves
136 blocks, but only 10 blocks go to the log. a commit is expensive
and correct reservation allows us to avoid needless commits. here
is the patch. tested on UP.

thanks, Alex


Signed-off-by: Alex Tomas <alex@clusterfs.com>

Index: linux-2.6.7/fs/jbd/transaction.c
===================================================================
--- linux-2.6.7.orig/fs/jbd/transaction.c	2004-08-26 17:12:40.000000000 +0400
+++ linux-2.6.7/fs/jbd/transaction.c	2005-01-19 17:23:30.058160408 +0300
@@ -1204,6 +1257,7 @@
 	transaction_t *transaction = handle->h_transaction;
 	journal_t *journal = transaction->t_journal;
 	struct journal_head *jh;
+	int drop_reserve = 0;
 
 	BUFFER_TRACE(bh, "entry");
 
@@ -1227,6 +1281,7 @@
 		J_ASSERT_JH(jh, !jh->b_committed_data);
 
 		__journal_unfile_buffer(jh);
+		drop_reserve = 1;
 
 		/* 
 		 * We are no longer going to journal this buffer.
@@ -1249,7 +1304,7 @@
 				spin_unlock(&journal->j_list_lock);
 				jbd_unlock_bh_state(bh);
 				__bforget(bh);
-				return;
+				goto drop;
 			}
 		}
 	} else if (jh->b_transaction) {
@@ -1264,6 +1319,7 @@
 		if (jh->b_next_transaction) {
 			J_ASSERT(jh->b_next_transaction == transaction);
 			jh->b_next_transaction = NULL;
+			drop_reserve = 1;
 		}
 	}
 
@@ -1271,6 +1327,15 @@
 	spin_unlock(&journal->j_list_lock);
 	jbd_unlock_bh_state(bh);
 	__brelse(bh);
+
+drop:
+	if (drop_reserve) {
+		/* no need to reserve log space for this block -bzzz */
+		spin_lock(&transaction->t_handle_lock);
+		transaction->t_outstanding_credits--;
+		spin_unlock(&transaction->t_handle_lock);
+	}
+
 	return;
 }
 

Re: [Ext2-devel] [PATCH] JBD: log space management optimization

[Stephen C. Tweedie]

Hi,

On Wed, 2005-01-19 at 15:32, Alex Tomas wrote:

> during truncate ext3 calls journal_forget() for freed blocks, but
> before these blocks go to the transaction and jbd reserves space
> in log for them (->t_outstanding_credits). also, journal_forget()
> removes these blocks from the transaction, but doesn't correct
> log space reservation. for example, removal of 500MB file reserves
> 136 blocks, but only 10 blocks go to the log. a commit is expensive
> and correct reservation allows us to avoid needless commits. here
> is the patch. tested on UP.

Looks like a good approach to me, but would it not be better to return
the credits to the handle instead of to the transaction?

A really large truncate will typically be getting a bunch of credits,
using those up and then extending itself continually as it encounters
more and more indirect blocks.

With your patch, the extended credits that the handle obtained will be
returned to the transaction, effectively shrinking the transaction again
and forcing the handle to extend itself yet again as it continues.  If
you returned them to the handle directly, it would be slightly more
efficient.

ACK either way, though --- the patch you've got now does look correct as
it stands.

Cheers,
 Stephen

Re: [Ext2-devel] [PATCH] JBD: log space management optimization

[Alex Tomas]

>>>>> Stephen C Tweedie (SCT) writes:

 SCT> If you returned them to the handle directly, it would be slightly more
 SCT> efficient.

good point. thanks. here is the fixed patch.


during truncate ext3 calls journal_forget() for freed blocks, but
before these blocks go to the transaction and jbd reserves space
in log for them (->t_outstanding_credits). also, journal_forget()
removes these blocks from the transaction, but doesn't correct
log space reservation. for example, removal of 500MB file reserves
136 blocks, but only 10 blocks go to the log. a commit is expensive
and correct reservation allows us to avoid needless commits. here
is the patch. tested on UP.



Signed-off-by: Alex Tomas <alex@clusterfs.com>
Index: linux-2.6.7/fs/jbd/transaction.c
===================================================================
--- linux-2.6.7.orig/fs/jbd/transaction.c	2004-08-26 17:12:40.000000000 +0400
+++ linux-2.6.7/fs/jbd/transaction.c	2005-01-24 22:51:34.000000000 +0300
@@ -1204,6 +1204,7 @@
 	transaction_t *transaction = handle->h_transaction;
 	journal_t *journal = transaction->t_journal;
 	struct journal_head *jh;
+	int drop_reserve = 0;
 
 	BUFFER_TRACE(bh, "entry");
 
@@ -1227,6 +1228,7 @@
 		J_ASSERT_JH(jh, !jh->b_committed_data);
 
 		__journal_unfile_buffer(jh);
+		drop_reserve = 1;
 
 		/* 
 		 * We are no longer going to journal this buffer.
@@ -1249,7 +1251,7 @@
 				spin_unlock(&journal->j_list_lock);
 				jbd_unlock_bh_state(bh);
 				__bforget(bh);
-				return;
+				goto drop;
 			}
 		}
 	} else if (jh->b_transaction) {
@@ -1264,6 +1266,7 @@
 		if (jh->b_next_transaction) {
 			J_ASSERT(jh->b_next_transaction == transaction);
 			jh->b_next_transaction = NULL;
+			drop_reserve = 1;
 		}
 	}
 
@@ -1271,6 +1274,13 @@
 	spin_unlock(&journal->j_list_lock);
 	jbd_unlock_bh_state(bh);
 	__brelse(bh);
+
+drop:
+	if (drop_reserve) {
+		/* no need to reserve log space for this block -bzzz */
+		handle->h_buffer_credits++;
+	}
+
 	return;
 }
 

Re: [Ext2-devel] [PATCH] JBD: log space management optimization

[Stephen C. Tweedie]

Hi,

On Mon, 2005-01-24 at 20:22, Alex Tomas wrote:

> during truncate ext3 calls journal_forget() for freed blocks, but
> before these blocks go to the transaction and jbd reserves space
> in log for them (->t_outstanding_credits). also, journal_forget()
> removes these blocks from the transaction, but doesn't correct
> log space reservation. for example, removal of 500MB file reserves
> 136 blocks, but only 10 blocks go to the log. a commit is expensive
> and correct reservation allows us to avoid needless commits. here
> is the patch. tested on UP.

> +drop:
> +	if (drop_reserve) {
> +		/* no need to reserve log space for this block -bzzz */
> +		handle->h_buffer_credits++;
> +	}
> +

Looks good to me.

--Stephen