On Wed 16-11-16 10:47:38, Ted Tso wrote: > On Mon, Nov 14, 2016 at 01:03:36PM -0800, Eric Biggers wrote: > > If the task actually were to wait for the journal to commit in this > > case, then it would deadlock because a handle remains open from > > __ext4_new_inode(), so the running transaction can't be committed yet. > > Fortunately, __jbd2_journal_force_commit() avoids the deadlock by not > > allowing the running transaction to be committed while the current task > > has it open. However, the above lockdep warning is still triggered. > > So this is a false positive introduced by > > 1eaa566d368b: jbd2: track more dependencies on transaction commit > > Instead of working around the problem here, perhaps it would be better > to fix __jbd2_journal_force_commit() so that it calls a newly created > __jbd2_log_wait_commit() which skips the jbd2_might_wait_for_commit() > (and then have jbd2_log_wait_commit call __jbd2_log_wait_commit with > the might_wait_for_commit check)? > > This isn't the only place where jbd2_journal_force_commit() is called > so if the problem is with the lockdep check, maybe we should just fix > the logic in the jbd2 layer, hmm? So it is a false positive in the sense that it is not a deadlock possibility. OTOH it is a valid report in the sense that the code does not do what it is intended to - i.e., force a transaction commit to free some blocks and retry block allocation. So I find the original fix actually worthwhile. Looking into ext4 codebase there are only 3 callsites of jbd2_journal_force_commit_nested() - all in some kind of retry loops and two of the call sites actually should not need the "nested" variant. Just ext4_should_retry_alloc() may be called when a transaction is held open and then the caller likely wanted something else anyway so it would be good to find these cases and fix them and then get rid of the "nested" variant of forcing a commit since that is easy to use in a wrong way. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
