On Thu, Feb 06, 2025 at 10:35:28AM +0000, John Garry wrote: > On 05/02/2025 19:50, Darrick J. Wong wrote: > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > > > index 59f7fc16eb80..580469668334 100644 > > > --- a/fs/xfs/xfs_reflink.c > > > +++ b/fs/xfs/xfs_reflink.c > > > @@ -786,35 +786,20 @@ xfs_reflink_update_quota( > > > * requirements as low as possible. > > > */ > > > STATIC int > > > -xfs_reflink_end_cow_extent( > > > +xfs_reflink_end_cow_extent_locked( > > > struct xfs_inode *ip, > > > xfs_fileoff_t *offset_fsb, > > > - xfs_fileoff_t end_fsb) > > > + xfs_fileoff_t end_fsb, > > > + struct xfs_trans *tp, > > > + bool *commit) > > Transactions usually come before the inode in the parameter list. > > ok > > > > > You don't need to pass out a @commit flag -- if the function returns > > nonzero then the caller has to cancel the transaction; otherwise it will > > return zero and the caller should commit it.> There's no penalty for > > committing a non-dirty transaction. > > If there is no penalty, then fine. But I don't feel totally comfortable with > this and would prefer to keep the same behavior. Right now this is the only place in XFS that behaves this way, which means you're adding a new code idiom that isn't present anywhere else in the code base. --D > Thanks, > John > > > >
