[PATCH 1/5] [XFS] Use xfs_sync_inodes() for device flushing

[Mikulas Patocka]

On Fri, 20 Mar 2009, Dave Chinner wrote:

> On Wed, Mar 18, 2009 at 12:14:51PM -0400, Mikulas Patocka wrote:
> > On Wed, 18 Mar 2009, Dave Chinner wrote:
> > > On Tue, Mar 17, 2009 at 09:08:36AM -0400, Mikulas Patocka wrote:
> > > > On Sun, 15 Mar 2009, Dave Chinner wrote:
> > What I find scary is those two recursive pagelocks being held without 
> > trylock. The sequence is like:
> > 
> > lock iolock 1
> > lock pagelock 1
> > --- submit request to xfssyncd, that does
> > trylock iolock 2
> > lock pagelock 2
> 
> Those two pages will be on different inodes, so locking through all
> paths to pagelock 2 except for page writeback will be protected by the iolocks...
> 
> > ... now suppose that this is racing with another process that takes 
> > pagelock without taking iolock first (memory manager trying to flush files 
> > mmaped for write or so). It can do
> > 
> > lock pagelock 2
> > ... and submit flush request to a thread that is actually waiting to get 
> > pagelock 2.
> 
> Which, AFAIK, is never done in XFS. Once we have a page locked in
> the writeback path we either dispatch it in an IO or unlock it
> directly before continuing. There should not be a case like you
> describe occurring (it is a bug if that does happen).
> 
> > --- I don't know --- is there a possibility to reserve filesystem space 
> > for write-mapped files at the time of the first page fault? (so that the 
> > space won't be allocated at the time of a flush?).
> 
> ->page_mkwrite

This is called without page lock so it is ok.

So I think there is no deadlock possibility.

But I still say that it is scary to take two pagelocks recursively and 
there is risk that someone will forget about these specific requirements 
after few years and make a bug.

Mikulas