| Message ID | 20180807120937.26597-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: use WRITE_ONCE to update if_seq | expand |
On Tue, Aug 07, 2018 at 02:09:37PM +0200, Christoph Hellwig wrote: > This adds ordering of the updates and makes sure we always see the if_seq > update before the extent tree is modified. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks reasonable enough... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_iext_tree.c | 20 +++++++++++++++++--- > fs/xfs/xfs_aops.c | 4 ++-- > fs/xfs/xfs_iomap.c | 3 ++- > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c > index 8a7aea041ee1..771dd072015d 100644 > --- a/fs/xfs/libxfs/xfs_iext_tree.c > +++ b/fs/xfs/libxfs/xfs_iext_tree.c > @@ -14,6 +14,7 @@ > #include "xfs_inode_fork.h" > #include "xfs_trans_resv.h" > #include "xfs_mount.h" > +#include "xfs_bmap.h" > #include "xfs_trace.h" > > /* > @@ -612,6 +613,19 @@ xfs_iext_realloc_root( > cur->leaf = new; > } > > +/* > + * Increment the sequence counter if we are on a COW fork. This allows > + * the writeback code to skip looking for a COW extent if the COW fork > + * hasn't changed. We use WRITE_ONCE here to ensure the update to the > + * sequence counter is seen before the modifications to the extent > + * tree itself take effect. > + */ > +static inline void xfs_iext_inc_seq(struct xfs_ifork *ifp, int state) > +{ > + if (state & BMAP_COWFORK) > + WRITE_ONCE(ifp->if_seq, READ_ONCE(ifp->if_seq) + 1); > +} > + > void > xfs_iext_insert( > struct xfs_inode *ip, > @@ -624,7 +638,7 @@ xfs_iext_insert( > struct xfs_iext_leaf *new = NULL; > int nr_entries, i; > > - ifp->if_seq++; > + xfs_iext_inc_seq(ifp, state); > > if (ifp->if_height == 0) > xfs_iext_alloc_root(ifp, cur); > @@ -866,7 +880,7 @@ xfs_iext_remove( > ASSERT(ifp->if_u1.if_root != NULL); > ASSERT(xfs_iext_valid(ifp, cur)); > > - ifp->if_seq++; > + xfs_iext_inc_seq(ifp, state); > > nr_entries = xfs_iext_leaf_nr_entries(ifp, leaf, cur->pos) - 1; > for (i = cur->pos; i < nr_entries; i++) > @@ -974,7 +988,7 @@ xfs_iext_update_extent( > { > struct xfs_ifork *ifp = xfs_iext_state_to_fork(ip, state); > > - ifp->if_seq++; > + xfs_iext_inc_seq(ifp, state); > > if (cur->pos == 0) { > struct xfs_bmbt_irec old; > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 235b4ddcd324..49f5f5896a43 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -351,7 +351,7 @@ xfs_map_blocks( > 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 == READ_ONCE(ip->i_cowfp->if_seq))) > return 0; > > if (XFS_FORCED_SHUTDOWN(mp)) > @@ -380,7 +380,7 @@ xfs_map_blocks( > xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap)) > cow_fsb = imap.br_startoff; > if (cow_fsb != NULLFILEOFF && cow_fsb <= offset_fsb) { > - wpc->cow_seq = ip->i_cowfp->if_seq; > + wpc->cow_seq = READ_ONCE(ip->i_cowfp->if_seq); > xfs_iunlock(ip, XFS_ILOCK_SHARED); > /* > * Truncate can race with writeback since writeback doesn't > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index 3282575e2df4..6320aca39f39 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -655,6 +655,7 @@ xfs_iomap_write_allocate( > unsigned int *cow_seq) > { > xfs_mount_t *mp = ip->i_mount; > + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork); > xfs_fileoff_t offset_fsb, last_block; > xfs_fileoff_t end_fsb, map_start_fsb; > xfs_filblks_t count_fsb; > @@ -768,7 +769,7 @@ xfs_iomap_write_allocate( > goto error0; > > if (whichfork == XFS_COW_FORK) > - *cow_seq = XFS_IFORK_PTR(ip, whichfork)->if_seq; > + *cow_seq = READ_ONCE(ifp->if_seq); > xfs_iunlock(ip, XFS_ILOCK_EXCL); > } > > -- > 2.18.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c index 8a7aea041ee1..771dd072015d 100644 --- a/fs/xfs/libxfs/xfs_iext_tree.c +++ b/fs/xfs/libxfs/xfs_iext_tree.c @@ -14,6 +14,7 @@ #include "xfs_inode_fork.h" #include "xfs_trans_resv.h" #include "xfs_mount.h" +#include "xfs_bmap.h" #include "xfs_trace.h" /* @@ -612,6 +613,19 @@ xfs_iext_realloc_root( cur->leaf = new; } +/* + * Increment the sequence counter if we are on a COW fork. This allows + * the writeback code to skip looking for a COW extent if the COW fork + * hasn't changed. We use WRITE_ONCE here to ensure the update to the + * sequence counter is seen before the modifications to the extent + * tree itself take effect. + */ +static inline void xfs_iext_inc_seq(struct xfs_ifork *ifp, int state) +{ + if (state & BMAP_COWFORK) + WRITE_ONCE(ifp->if_seq, READ_ONCE(ifp->if_seq) + 1); +} + void xfs_iext_insert( struct xfs_inode *ip, @@ -624,7 +638,7 @@ xfs_iext_insert( struct xfs_iext_leaf *new = NULL; int nr_entries, i; - ifp->if_seq++; + xfs_iext_inc_seq(ifp, state); if (ifp->if_height == 0) xfs_iext_alloc_root(ifp, cur); @@ -866,7 +880,7 @@ xfs_iext_remove( ASSERT(ifp->if_u1.if_root != NULL); ASSERT(xfs_iext_valid(ifp, cur)); - ifp->if_seq++; + xfs_iext_inc_seq(ifp, state); nr_entries = xfs_iext_leaf_nr_entries(ifp, leaf, cur->pos) - 1; for (i = cur->pos; i < nr_entries; i++) @@ -974,7 +988,7 @@ xfs_iext_update_extent( { struct xfs_ifork *ifp = xfs_iext_state_to_fork(ip, state); - ifp->if_seq++; + xfs_iext_inc_seq(ifp, state); if (cur->pos == 0) { struct xfs_bmbt_irec old; diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c index 235b4ddcd324..49f5f5896a43 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -351,7 +351,7 @@ xfs_map_blocks( 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 == READ_ONCE(ip->i_cowfp->if_seq))) return 0; if (XFS_FORCED_SHUTDOWN(mp)) @@ -380,7 +380,7 @@ xfs_map_blocks( xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap)) cow_fsb = imap.br_startoff; if (cow_fsb != NULLFILEOFF && cow_fsb <= offset_fsb) { - wpc->cow_seq = ip->i_cowfp->if_seq; + wpc->cow_seq = READ_ONCE(ip->i_cowfp->if_seq); xfs_iunlock(ip, XFS_ILOCK_SHARED); /* * Truncate can race with writeback since writeback doesn't diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 3282575e2df4..6320aca39f39 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -655,6 +655,7 @@ xfs_iomap_write_allocate( unsigned int *cow_seq) { xfs_mount_t *mp = ip->i_mount; + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork); xfs_fileoff_t offset_fsb, last_block; xfs_fileoff_t end_fsb, map_start_fsb; xfs_filblks_t count_fsb; @@ -768,7 +769,7 @@ xfs_iomap_write_allocate( goto error0; if (whichfork == XFS_COW_FORK) - *cow_seq = XFS_IFORK_PTR(ip, whichfork)->if_seq; + *cow_seq = READ_ONCE(ifp->if_seq); xfs_iunlock(ip, XFS_ILOCK_EXCL); }
This adds ordering of the updates and makes sure we always see the if_seq update before the extent tree is modified. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_iext_tree.c | 20 +++++++++++++++++--- fs/xfs/xfs_aops.c | 4 ++-- fs/xfs/xfs_iomap.c | 3 ++- 3 files changed, 21 insertions(+), 6 deletions(-)