Re: [RFC] Bug in journal_commit_transaction?

["Stephen C. Tweedie" <sct@redhat.com>]

Hi,

On Thu, 2006-04-20 at 10:47 +0200, Jan Kara wrote:

>   I've got no answer but I still think my argument was sound ;) Below
> is a patch also with a verbose comment what it fixes and how.

Looks good to me, but it complicates the logic for this case and removes
some useful debug checks for conditions which are almost always true.

Wouldn't it be better and safer just to make those almost-always true
conditions always true?  We can do that pretty simply, just by
processing the t_forget list in two passes, always doing the bitmaps
last.  (Though this will need care, as the list is potentially moving
under our feet...)

Actually, scratch that, because journal_unmap_buffer() can return a
buffer to the committing transaction's forget list at any time, so we
really cannot guarantee to do all non-bitmaps first: new non-bitmaps
might arrive after we've started doing the bitmaps.  Ugh.

> diff -rupX /home/jack/.kerndiffexclude linux-2.6.16/fs/jbd/commit.c linux-2.6.16-1-realloc_freed_fix/fs/jbd/commit.c
> --- linux-2.6.16/fs/jbd/commit.c	2006-01-15 00:20:12.000000000 +0100
> +++ linux-2.6.16-1-realloc_freed_fix/fs/jbd/commit.c	2006-04-20 10:32:15.000000000 +0200
> @@ -790,11 +790,22 @@ restart_loop:
> +			JBUFFER_TRACE(jh, "refile or unfile freed buffer");
> +			__journal_refile_buffer(jh);
> +			if (!jh->b_transaction) {
> +				jbd_unlock_bh_state(bh);
> +				 /* needs a brelse */
> +				journal_remove_journal_head(bh);
> +				release_buffer_page(bh);
> +			} else
> +				jbd_unlock_bh_state(bh);

Looks good to me.  ACK.

--Stephen