Re: Lock ordering in iomap code

[Dave Chinner <david@fromorbit.com>]

On Thu, Oct 20, 2016 at 01:55:00PM +0200, Jan Kara wrote:
> Hi Christoph,
> 
> On Mon 17-10-16 12:26:57, Christoph Hellwig wrote:
> > > Ping? I have ext4 DAX read & write path working with the iomap code but to
> > > convert the fault path, I need this resolved. Are you OK with moving
> > > iomap_begin() / iomap_end() calls outside of page lock / entry lock in the
> > > fault path?
> > 
> > Yes, that sounds fine.
> 
> I've been looking into this some more and realized it's not as easy as I've
> originally though. ->page_mkwrite callback is expected to return with the
> page locked so locking it inside the iomap actor is really awkward (think
> of situation when blocksize < pagesize). Since nobody currently has issues
> with ->iomap_begin being sometimes called with page lock and sometimes
> without, I don't think changing that would be worth the hassle. Just one
> more question: Doesn't XFS have some lock ordering issues when
> xfs_file_iomap_begin() gets called with page lock held from
> iomap_page_mkwrite() for a file with extent size hints and thus we end up
> in xfs_iomap_write_direct() with page lock held? We do a lot of stuff there
> including transaction setup and such...

Lock order in xfs is iolock->page lock->transaction->ilock, so what
is being done in xfs_file_iomap_begin (transactions, ilock) is fine
regardless of whether we hold the page locked or noti when called.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com