From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
In Christoph Hellwig's patch "xfs: avoid COW fork extent lookups in
writeback if the fork didn't change" (which has not yet graduated to
for-next), we sample the COW fork sequence number without taking the
ilock. This is a little strange, since in general we always take it
before accessing anything in a block mapping. I think we get lucky in
that the unlocking during actual cow fork changes will erect the
necessary memory barriers (on x86 anyway) but let's not play fast and
loose with breaking everyone else's model of how locking works.
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
---
fs/xfs/xfs_aops.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index aff9d44fa338..2e178ef89a15 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -338,16 +338,21 @@ xfs_map_blocks(
* COW one, or the COW fork hasn't changed from the last time we looked
* at it.
*/
+ xfs_ilock(ip, XFS_ILOCK_SHARED);
imap_valid = offset_fsb >= wpc->imap.br_startoff &&
offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
if (imap_valid &&
(!xfs_inode_has_cow_data(ip) ||
wpc->io_type == XFS_IO_COW ||
- wpc->cow_seq == ip->i_cowfp->if_seq))
+ wpc->cow_seq == ip->i_cowfp->if_seq)) {
+ xfs_iunlock(ip, XFS_ILOCK_SHARED);
return 0;
+ }
- if (XFS_FORCED_SHUTDOWN(mp))
+ if (XFS_FORCED_SHUTDOWN(mp)) {
+ xfs_iunlock(ip, XFS_ILOCK_SHARED);
return -EIO;
+ }
/*
* If we don't have a valid map, now it's time to get a new one for this
@@ -355,7 +360,6 @@ xfs_map_blocks(
* into real extents. If we return without a valid map, it means we
* landed in a hole and we skip the block.
*/
- xfs_ilock(ip, XFS_ILOCK_SHARED);
ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
(ip->i_df.if_flags & XFS_IFEXTENTS));
ASSERT(offset <= mp->m_super->s_maxbytes);
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
