Hello, sorry, I was probably to terse in my previous mail. I'll try to describe more details now. > > I think I have found a possible cause of assertion failure in > > journal_commit_buffer() on line 793 (b_next_transaction == NULL) (which > > you mean journal_commit_transaction(). Yes. > > was reported about two times as I remember). And I think the problem is > > just a few lines below in cond_resched_lock(&journal->j_list_lock); > > The problem is following: we are processing t_forget list of a > > committing transaction. On this forget list are among other buffers also > > bitmaps and buffers that are freed by this transaction. Now if bitmap > > buffer is processed first, we switch to new bitmap (hence the buffer > > freed by this transaction is again available for allocation). If we > > reschedule on that cond_resched_lock() and in the meantime someone > > allocates the buffer, we later fail with that assertion failure. > > I assume you're referring to the cond_resched_lock() at line 799. > > I don't really follow your description here (it seems to be missing bits), > but that loop drops that lock in other places - why wouldn't those places > also be vulnerable? Yes, I meant that cond_resched_lock(). But you are right that the problem is not in cond_resched_lock() itself. It is in the pure fact that we can process bitmap buffer in t_forget list before all the buffers transaction has freed. Explanation is below. When we are freeing some blocks we do journal_get_undo_access() on the bitmap buffer. That copies current state of the bitmap into b_committed_data. When we allocate any new blocks we check that the block is free in both b_committed_data (old state) and in b_data (new state). That makes sure that we do not reallocate any freed block before transaction that frees it is committed. But unlocking j_list_lock when processing t_forget list in journal_commit_transaction() can actually break that assumption in some cases: If we commit the transaction we write the bitmap buffer into the log and then file it into BJ_Forget list. We also write the freed data buffer (e.g. zeroed indirect block) and file it into BJ_Forget list. Then we start processing BJ_Forget list. First we find the bitmap buffer. In line 748 we free the b_committed_data stored by journal_get_undo_access(). From now on the allocation code in ext3_test_allocatable() will no longer find b_committed_data and hence it will assume the block is freed. *BUT* we still have the freed buffer in t_forget list. If it happens that we reallocate the block before we process the buffer in t_forget list, it will have b_next_transaction set and we fail at the assertion failure in line 793. I hope it is clearer now. Honza -- Jan Kara <jack@xxxxxxx> SuSE CR Labs - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
