Re: unmount oops in log_do_checkpoint

[Jan Kara <jack@suse.cz>]

> On Tue, Jan 17, 2006 at 05:32:35PM +0100, Jan Kara wrote:
> > > On Tue, Jan 17, 2006 at 12:59:45PM +0100, Nick Piggin wrote:
> > > 
> > > Maybe it is because people haven't been turning on their debugging options,
> > > tsk tsk ;) It only oopses when DEBUG_SLAB and DEBUG_PAGEALLOC are both
> > > enabled. And only then when the jbd patch is not reverted. Weird.
> >   Hmm, that's really strange, maybe we have some use-after-free
> > problem or so... I'll see what I can do :).
> > 
> 
> Are you able to reproduce? If not I can test patches...
  Hmm, I was not able to reproduce the problem even with those debug
options set :(. As I'm looking into the code it seems that somebody
managed to free the transaction but did not clear the
j_checkpoint_transactions pointer. It's even stranger that it's during
umount time when there should not be much processes playing with the JBD
structures on that filesystem.
  Attached is the patch that fixes two minor possible problems I've
found. Neither of them should be causing your oops but one never knows
:). Also turn on the JBD debugging in config. Maybe it spits something
useful. If you still see the same oops, I'll create some debugging
patch.
  BTW: the oops during umount is after some activity on the filesystem
or you just mount & umount?

						Thanks for testing
								Honza

-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

diff -rupX /home/jack/.kerndiffexclude linux-2.6.16-rc1/fs/jbd/checkpoint.c linux-2.6.16-rc1-1-checkpoint-fix/fs/jbd/checkpoint.c
--- linux-2.6.16-rc1/fs/jbd/checkpoint.c	2006-01-17 21:44:02.000000000 +0100
+++ linux-2.6.16-rc1-1-checkpoint-fix/fs/jbd/checkpoint.c	2006-01-17 23:35:49.000000000 +0100
@@ -338,7 +338,7 @@ restart:
 	 * done (maybe it's a new transaction, but it fell at the same
 	 * address).
 	 */
- 	if (journal->j_checkpoint_transactions == transaction ||
+ 	if (journal->j_checkpoint_transactions == transaction &&
 			transaction->t_tid == this_tid) {
 		int batch_count = 0;
 		struct buffer_head *bhs[NR_BATCH];
diff -rupX /home/jack/.kerndiffexclude linux-2.6.16-rc1/fs/jbd/commit.c linux-2.6.16-rc1-1-checkpoint-fix/fs/jbd/commit.c
--- linux-2.6.16-rc1/fs/jbd/commit.c	2006-01-15 00:20:12.000000000 +0100
+++ linux-2.6.16-rc1-1-checkpoint-fix/fs/jbd/commit.c	2006-01-17 23:35:19.000000000 +0100
@@ -829,7 +829,8 @@ restart_loop:
 	journal->j_committing_transaction = NULL;
 	spin_unlock(&journal->j_state_lock);
 
-	if (commit_transaction->t_checkpoint_list == NULL) {
+	if (commit_transaction->t_checkpoint_list == NULL &&
+	    commit_transaction->t_checkpoint_io_list == NULL) {
 		__journal_drop_transaction(journal, commit_transaction);
 	} else {
 		if (journal->j_checkpoint_transactions == NULL) {