| Message ID | 20200721001606.10781-15-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: Delay Ready Attributes | expand |
On Mon, Jul 20, 2020 at 05:15:55PM -0700, Allison Collins wrote: > A transaction roll is not necessary immediately after setting the > INCOMPLETE flag when removing a node xattr entry with remote value > blocks. The remote block invalidation that immediately follows setting > the flag is an in-core only change. The next step after that is to start > unmapping the remote blocks from the attr fork, but the xattr remove > transaction reservation includes reservation for full tree splits of the > dabtree and bmap tree. The remote block unmap code will roll the > transaction as extents are unmapped and freed. > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > Reviewed-by: Brian Foster <bfoster@redhat.com> Urrrk. The analysis is correct here, but whoooee was it hard to find. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_attr.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 1a78023..f1becca 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1148,10 +1148,6 @@ xfs_attr_node_removename( > if (error) > goto out; > > - error = xfs_trans_roll_inode(&args->trans, args->dp); > - if (error) > - goto out; > - > error = xfs_attr_rmtval_invalidate(args); > if (error) > return error; > -- > 2.7.4 >
On 7/21/20 4:38 PM, Darrick J. Wong wrote: > On Mon, Jul 20, 2020 at 05:15:55PM -0700, Allison Collins wrote: >> A transaction roll is not necessary immediately after setting the >> INCOMPLETE flag when removing a node xattr entry with remote value >> blocks. The remote block invalidation that immediately follows setting >> the flag is an in-core only change. The next step after that is to start >> unmapping the remote blocks from the attr fork, but the xattr remove >> transaction reservation includes reservation for full tree splits of the >> dabtree and bmap tree. The remote block unmap code will roll the >> transaction as extents are unmapped and freed. >> >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> Reviewed-by: Brian Foster <bfoster@redhat.com> > > Urrrk. The analysis is correct here, but whoooee was it hard to find. I know, its a lot of explaining for what looks like such a small change, but better to have it than not I think > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Thank you! Allison > > --D > >> --- >> fs/xfs/libxfs/xfs_attr.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 1a78023..f1becca 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1148,10 +1148,6 @@ xfs_attr_node_removename( >> if (error) >> goto out; >> >> - error = xfs_trans_roll_inode(&args->trans, args->dp); >> - if (error) >> - goto out; >> - >> error = xfs_attr_rmtval_invalidate(args); >> if (error) >> return error; >> -- >> 2.7.4 >>
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 1a78023..f1becca 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1148,10 +1148,6 @@ xfs_attr_node_removename( if (error) goto out; - error = xfs_trans_roll_inode(&args->trans, args->dp); - if (error) - goto out; - error = xfs_attr_rmtval_invalidate(args); if (error) return error;