On Fri 18-05-12 20:28:29, Dave Chinner wrote:
> On Thu, May 17, 2012 at 10:08:31PM +0200, Jan Kara wrote:
> > Hello,
> > I've been into a source of lockdep warning I got with XFS when testing my
> > filesystem freezing patches. The culprit seems to be that when doing direct
> > IO, XFS starts a transaction in xfs_setfilesize_trans_alloc() and attaches
> > that transaction to endio structure. Then it goes on and starts another
> > transaction in xfs_iomap_write_direct() which creates a possible deadlock
> > with filesystem freezing (if the second transaction start happens after we
> > start blocking new transactions, it gets blocked, but the first transaction
> > isn't ever completed).
> Both occur within the context of an active write IO - why would
> new transactions be blocked while there are still active write
> operations that require allocation transactions occurring?
Yeah, you are right that sb_start_write() will actually protect the
transaction start so the deadlock with freezing cannot really occur.
> > So first I wanted to ask whether my analysis is correct. If yes, I was also
> > wondering whether this cannot cause a deadlock (at least in theory) if the
> > second transaction would block waiting for log space but we couldn't
> > possibly free enough of it due to the first transaction being held open?
> Don't think so. The first transaction reservation is for the inode
> size update, but it doesn't hold anything locked so it will not hold
> up log tail pushing so the second transaction reservation will not
> get blocked by it. The onyl way that could happen is if the
> combination of the two transactions is greater than 25% of the log,
> and given that the size update transaction reservation is only about
> 600 bytes, that can't occur....
OK, I see. I was thinking that it's likely OK but I was wondering about
the details and couldn't quite figure it out from reading the transaction
> > If freezing deadlock is the only problem with this code, then I guess we
> > could avoid waiting for filesystem freezing when starting the second
> > transaction (although it might end up being rather ugly). Or if anyone else
> > has other idea how to solve this, I'm listening ;).
> I'm confused about why the active sb_start_write() of the direct IO
> wouldn't hold off the freeze until the IO has completed. That should
> completely protect the write against freeze until the IO completes,
> which AFAICT means the lockdep report is a false positive....
Yes, it will, you are right. I was too tired to realize this yesterday.
So I just have to figure out how to properly instrument lockdep to avoid
these warnings. Thanks for having a look!
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR