[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/5] xfs: use xfs_ilock_map_shared in xfs_qm_dqtobp
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 5 Dec 2013 12:53:45 -0800
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131205204108.GB29897@dastard>
References: <20131205155830.620826868@xxxxxxxxxxxxxxxxxxxxxx> <20131205155951.330689967@xxxxxxxxxxxxxxxxxxxxxx> <20131205204108.GB29897@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
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..

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