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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
