xfs
[Top] [All Lists]

Re: Are two transactions running in parallel OK?

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: Are two transactions running in parallel OK?
From: Jan Kara <jack@xxxxxxx>
Date: Fri, 18 May 2012 15:21:09 +0200
Cc: Jan Kara <jack@xxxxxxx>, xfs@xxxxxxxxxxx, sandeen@xxxxxxxxxx, dchinner@xxxxxxxxxx
In-reply-to: <20120518102829.GY25351@dastard>
References: <20120517200831.GB23231@xxxxxxxxxxxxx> <20120518102829.GY25351@dastard>
User-agent: Mutt/1.5.20 (2009-06-14)
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
code.

> > 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!

                                                                Honza
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR

<Prev in Thread] Current Thread [Next in Thread>