xfs
[Top] [All Lists]

Re: [PATCH v2 1/3] xfs: add scan owner field to xfs_eofblocks

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v2 1/3] xfs: add scan owner field to xfs_eofblocks
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 28 May 2014 07:26:53 +1000
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140527121810.GB63281@xxxxxxxxxxxxxxx>
References: <1400845950-41435-1-git-send-email-bfoster@xxxxxxxxxx> <1400845950-41435-2-git-send-email-bfoster@xxxxxxxxxx> <20140527104428.GC1440@xxxxxxxxxxxxx> <20140527121810.GB63281@xxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, May 27, 2014 at 08:18:11AM -0400, Brian Foster wrote:
> On Tue, May 27, 2014 at 03:44:28AM -0700, Christoph Hellwig wrote:
> > On Fri, May 23, 2014 at 07:52:28AM -0400, Brian Foster wrote:
> > > The scan owner field represents an optional inode number that is
> > > responsible for the current scan. The purpose is to identify that an
> > > inode is under iolock and as such, the iolock shouldn't be attempted
> > > when trimming eofblocks. This is an internal only field.
> > 
> > xfs_free_eofblocks already does a trylock, and without that calling it
> > from one iolock holding process to another would be a deadlock waiting
> > to happen.
> > 
> > I have to say I'm still not very easy with iolock nesting, even if it's
> > a trylock.
> > 
> 
> Right... maybe I'm not parsing your point. The purpose here is to avoid
> the trylock entirely. E.g., Indicate that we have already acquired the
> lock and can proceed with xfs_free_eofblocks(), rather than fail a
> trylock and skip (which appears to be a potential infinite loop scenario
> here due to how the AG walking code handles EAGAIN).

I think Christoph's concern here is that we are calling a function
that can take the iolock while we already hold the iolock. i.e. the
reason we have to add the anti-deadlock code in the first place.  To
address that, can we restructure xfs_file_buffered_aio_write() such
that the ENOSPC/EDQUOT flush is done outside the iolock?

>From a quick check, I don't think there is any problem with dropping
the iolock, doing the flushes and then going all the way back to the
start of the function again, but closer examination and testing is
warranted...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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