On Aug 29, 2005 18:40 +0100, Matthew Wilcox wrote:
> journal_commit_transaction() is still 650+ lines long and contains 16
> local variables. By moving phase 3 into its own function, we reduce
> its length by 150+ lines and reduce it to 5 local variables.
>
> @@ -251,6 +424,7 @@ void journal_commit_transaction(journal_
> * buffer are perfectly permissable.
> */
> while (commit_transaction->t_reserved_list) {
> + struct journal_head *jh;
> jh = commit_transaction->t_reserved_list;
> JBUFFER_TRACE(jh, "reserved, unused: refile");
> /*
> @@ -301,6 +475,7 @@ void journal_commit_transaction(journal_
> */
> spin_lock(&journal->j_list_lock);
> if (commit_transaction->t_buffers) {
> + struct journal_head *jh, *new_jh;
> new_jh = jh = commit_transaction->t_buffers->b_tnext;
> do {
> J_ASSERT_JH(new_jh, new_jh->b_modified == 1 ||
> @@ -333,6 +508,7 @@ write_out_data:
>
> while (commit_transaction->t_sync_datalist) {
> struct buffer_head *bh;
> + struct journal_head *jh;
>
> jh = commit_transaction->t_sync_datalist;
> commit_transaction->t_sync_datalist = jh->b_tnext;
The one comment I have here is that in the past GCC has been bad at
allocating local variables on the stack, and would reserve the sum
of all local variables instead of the union. This would unnecessarily
bloat code that had a lot of local variables defined in only a subset
of the function. Could you verify with checkstack that this doesn't hit
here? Otherwise we need to move the common journal_head and buffer_head
allocations up to the main function declarations.
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
