[PATCH] kjournald() missing JFS_UNMOUNT check

[PATCH] kjournald() missing JFS_UNMOUNT check

[Mark Fasheh]

Hi,
	Can we please merge this patch? I sent it to ext2-devel for comments
last week and haven't hear anything back. It seems trivially correct and is
testing fine - famous last words, I know :)

My original mail follows:

	It seems that kjournald() may be missing a check of the JFS_UNMOUNT
flag before calling schedule(). This showed up in testing of OCFS2 recovery
where our recovery thread would hang in journal_kill_thread() called from  
journal_destroy() because kjournald never got a chance to read the flag to 
shut down before the schedule().

Zach pointed out the missing check which led me to hack up this trivial
patch. It's been tested many times now and I have yet to reproduce the 
hang, which was happening very regularly before.

<mild rant>
I'm guessing that we could really use some wait_event() calls with helper
functions in, well, most of jbd these days which would make a ton of the 
wait code there vastly cleaner.
</mild rant>

As for why this doesn't happen in ext3 (or OCFS2 during normal mount/unmount
of the local nodes journal), I think it may that the specific timing of
events in the ocfs2 recovery thread exposes a race there. Because
ocfs2_replay_journal() is only interested in playing back the journal,
initialization and shutdown happen very quicky with no other metadata put
into that specific journal.
	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh@oracle.com

--- ../linux-2.6.13-rc1-ro/fs/jbd/journal.c	2005-06-28 22:57:29.000000000 -0700
+++ linux-2.6.13-rc1/fs/jbd/journal.c	2005-07-07 12:01:44.280714000 -0700
@@ -193,6 +193,8 @@
 		if (transaction && time_after_eq(jiffies,
 						transaction->t_expires))
 			should_sleep = 0;
+		if (journal->j_flags & JFS_UNMOUNT)
+ 			should_sleep = 0;
 		if (should_sleep) {
 			spin_unlock(&journal->j_state_lock);
 			schedule();

Re: [PATCH] kjournald() missing JFS_UNMOUNT check

[Stephen C. Tweedie]

Hi,

On Mon, 2005-07-11 at 22:19, Mark Fasheh wrote:

> 	Can we please merge this patch? I sent it to ext2-devel for comments
> last week and haven't hear anything back. It seems trivially correct and is
> testing fine - famous last words, I know :)

ACK --- looks fine to me.

--Stephen