| Message ID | 20180717232405.18511-6-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
> struct xfs_ifork { > int if_bytes; /* bytes in if_u1 */ > + unsigned int if_seq; I wonder if a comment here wouldn't be helpful in the future, like: /* ext list modification counter */ It's just a suggestion though, any way you can add: Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > struct xfs_btree_block *if_broot; /* file's incore btree root */ > short if_broot_bytes; /* bytes allocated for root */ > unsigned char if_flags; /* per-fork flags */ > -- > 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
On Wed, Jul 18, 2018 at 04:40:04PM +0200, Carlos Maiolino wrote: > > > > struct xfs_ifork { > > int if_bytes; /* bytes in if_u1 */ > > + unsigned int if_seq; > > I wonder if a comment here wouldn't be helpful in the future, like: /* ext list modification counter */ Does that really add more value? If so I'm happy having it added when applying. -- 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
On Thu, Jul 19, 2018 at 06:32:33PM +0200, Christoph Hellwig wrote: > On Wed, Jul 18, 2018 at 04:40:04PM +0200, Carlos Maiolino wrote: > > > > > > > struct xfs_ifork { > > > int if_bytes; /* bytes in if_u1 */ > > > + unsigned int if_seq; > > > > I wonder if a comment here wouldn't be helpful in the future, like: /* ext list modification counter */ > > Does that really add more value? If so I'm happy having it added > when applying. Seeing as we have anecdotal evidence that more work will be needed if we ever want to use it to track change count in the data fork, I think I'll add the following comment: unsigned int if_seq; /* cow fork mod count */ --D > -- > 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
On Thu, Jul 19, 2018 at 11:27:16AM -0700, Darrick J. Wong wrote: > On Thu, Jul 19, 2018 at 06:32:33PM +0200, Christoph Hellwig wrote: > > On Wed, Jul 18, 2018 at 04:40:04PM +0200, Carlos Maiolino wrote: > > > > > > > > > > struct xfs_ifork { > > > > int if_bytes; /* bytes in if_u1 */ > > > > + unsigned int if_seq; > > > > > > I wonder if a comment here wouldn't be helpful in the future, like: /* ext list modification counter */ > > > > Does that really add more value? If so I'm happy having it added > > when applying. > For me at least it adds, IMHO, it's simpler to see a small comment in the struct definition other than search what exactly it's supposed to mean in the implementation, but well, this is my view. > Seeing as we have anecdotal evidence that more work will be needed if we > ever want to use it to track change count in the data fork, I think I'll > add the following comment: > > unsigned int if_seq; /* cow fork mod count */ > /me didn't understand, but *shurg* > --D > > > -- > > 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 b80c63faace2..8a7aea041ee1 100644 --- a/fs/xfs/libxfs/xfs_iext_tree.c +++ b/fs/xfs/libxfs/xfs_iext_tree.c @@ -624,6 +624,8 @@ xfs_iext_insert( struct xfs_iext_leaf *new = NULL; int nr_entries, i; + ifp->if_seq++; + if (ifp->if_height == 0) xfs_iext_alloc_root(ifp, cur); else if (ifp->if_height == 1) @@ -864,6 +866,8 @@ xfs_iext_remove( ASSERT(ifp->if_u1.if_root != NULL); ASSERT(xfs_iext_valid(ifp, cur)); + ifp->if_seq++; + nr_entries = xfs_iext_leaf_nr_entries(ifp, leaf, cur->pos) - 1; for (i = cur->pos; i < nr_entries; i++) leaf->recs[i] = leaf->recs[i + 1]; @@ -970,6 +974,8 @@ xfs_iext_update_extent( { struct xfs_ifork *ifp = xfs_iext_state_to_fork(ip, state); + ifp->if_seq++; + if (cur->pos == 0) { struct xfs_bmbt_irec old; diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h index 1492143371f3..f20b2468ca35 100644 --- a/fs/xfs/libxfs/xfs_inode_fork.h +++ b/fs/xfs/libxfs/xfs_inode_fork.h @@ -14,6 +14,7 @@ struct xfs_dinode; */ struct xfs_ifork { int if_bytes; /* bytes in if_u1 */ + unsigned int if_seq; struct xfs_btree_block *if_broot; /* file's incore btree root */ short if_broot_bytes; /* bytes allocated for root */ unsigned char if_flags; /* per-fork flags */
Add a simple 32-bit unsigned integer as the sequence count for modifications to the extent list in the inode fork. This will be used to optimize away extent list lookups in the writeback code. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_iext_tree.c | 6 ++++++ fs/xfs/libxfs/xfs_inode_fork.h | 1 + 2 files changed, 7 insertions(+)