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

Christoph Hellwig
2013-12-05T20:53:47Z

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..