Re: [PATCH 2/5] xfs: use xfs_ilock_map_shared in xfs_qm_dqtobp

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 05, 2013 at 12:53:45PM -0800, Christoph Hellwig wrote:
> On Fri, Dec 06, 2013 at 07:41:08AM +1100, Dave Chinner wrote:
> > However, it raises a bigger question about dquot allocation sanity
> > to me: what makes the map returned valid after we've unlocked the
> > extent list?
> > 
> > We then use it to determine whether to allocate a
> > dquot or not, and xfs_qm_dqalloc() then does this after calling
> > xfs_bmapi_write():
> > 
> > 	ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> > 	       (map.br_startblock != HOLESTARTBLOCK));
> > 
> > What's to prevent someone coming in between the xfs_bmapi_read()
> > and *write() calls and allocating a different dquot in the same
> > cluster and therefore beating the first thread to the allocation?
> > 
> > This read/write race exists elsewhere - e.g. xfs_iomap_write_allocate
> > documents it for the data path - and it has to be specifically
> > handled to prevent corruption.....
> 
> Yeah, we'll need to read-read the extent map in xfs_qm_dqalloc. I  I
> think this is efficiently paper over by the buffer lock - we take
> it right after the xfs_bmapi_write over the period of initialization
> the on-disk dquots and copying the one we were called for into the
> in-memory one.  So while we might have been corrupting dquots all
> over no one has noticed because we had a non-corrupted version
> in-memory that overwrote the corrupted one again later.  Uhh..

Hmmm - I didn't think of the "rewrite dquot form memory into buffer"
aspect of it. That makes it even more subtle and less likely to be
seen unless we crash at exactly the wrong time...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs