Re: [RFC PATCH] xfs: fix cow_seq locking behavior

[Christoph Hellwig <hch@infradead.org>]

On Mon, Jul 30, 2018 at 08:52:07AM -0700, Darrick J. Wong wrote:
> /*
>  * COW fork blocks can overlap data fork blocks even if the blocks
>  * aren't shared.  COW I/O always takes precedent, so we must always
>  * check for overlap on reflink inodes unless the mapping is already a
>  * COW one.
>  *
>  * It's safe to check the COW fork if_seq here without the ILOCK because
>  * we've indirectly protected against concurrent updates: writeback has
>  * the page locked, which prevents concurrent invalidations by reflink
>  * and directio and prevents concurrent buffered writes to the same
>  * page.  Concurrent changes to other parts of the COW fork will drop
>  * the i_lock on their way out, which provides the necessary memory
>  * barrier to ensure that we see the updated if_seq.
>  */
> 
> I'm not actually sure about the last sentence anymore -- that's what I
> was thinking the first time I looked at this patch, before Dave spoke
> up.

Yeah, that last sentence looks odd.  I'd replace it with:

Changes to if_seq always happen under i_lock, which protects against
concurrent updates and provides a memory barrier on the way out that
ensures that we always see the current value.