On Wed, May 09, 2018 at 06:12:04AM -0400, Brian Foster wrote: > On Wed, May 09, 2018 at 10:40:59AM +1000, Dave Chinner wrote: > > On Tue, May 08, 2018 at 10:18:26AM -0400, Brian Foster wrote: > > > On Tue, May 08, 2018 at 01:42:00PM +1000, Dave Chinner wrote: > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > > > Another assert failure: > > > > > > > > XFS: Assertion failed: !(lip->li_flags & XFS_LI_TRANS), file: fs/xfs/xfs_trans.c, line: 740 > > > > > > Same assert comment... > > > > > > > .... > > > > xfs_trans_add_item+0xcc/0xe0 > > > > xfs_reflink_clear_inode_flag+0x53/0x120 > > > > xfs_reflink_try_clear_inode_flag+0x5b/0xa0 > > > > ? filemap_write_and_wait+0x4f/0x70 > > > > xfs_reflink_unshare+0x18e/0x19d > > > > xfs_file_fallocate+0x241/0x310 > > > > ? selinux_file_permission+0xd4/0x140 > > > > vfs_fallocate+0x13d/0x260 > > > > SyS_fallocate+0x43/0x80 > > > > > > > > Another fix. > > > > > > > > Signed-Off-By: Dave Chinner <dchinner@xxxxxxxxxx> > > > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > > > > --- > > > > fs/xfs/xfs_reflink.c | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > > > > index bce2b5351d64..12d441a73b53 100644 > > > > --- a/fs/xfs/xfs_reflink.c > > > > +++ b/fs/xfs/xfs_reflink.c > > > > @@ -1553,7 +1553,12 @@ xfs_reflink_inode_has_shared_extents( > > > > return 0; > > > > } > > > > > > > > -/* Clear the inode reflink flag if there are no shared extents. */ > > > > +/* > > > > + * Clear the inode reflink flag if there are no shared extents. > > > > + * > > > > + * The caller is responsible for joining the inode to the transaction passed in. > > > > + * The inode will be joined to the transaction that is returned to the caller. > > > > + */ > > > > int > > > > xfs_reflink_clear_inode_flag( > > > > struct xfs_inode *ip, > > > > @@ -1572,7 +1577,6 @@ xfs_reflink_clear_inode_flag( > > > > * We didn't find any shared blocks so turn off the reflink flag. > > > > * First, get rid of any leftover CoW mappings. > > > > */ > > > > - xfs_trans_ijoin(*tpp, ip, 0); > > > > ... > > > > > This seems a bit > > > superfluous. If the inode was already joined by the one caller of this > > > function, why not just remove this call in the previous patch rather > > > than move it and remove it? > > > > Because every time I put multiple independent fixes into a single > > patch I get told to separate them into individual patches. > > > > Ok, fair enough: > > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> Revised commit log: "xfs_reflink_clear_inode_flag double-joins an inode to a transaction, which is not allowed. Fix that and document that the caller must have already joined it." Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > > > Cheers, > > > > Dave. > > -- > > Dave Chinner > > david@xxxxxxxxxxxxx > > -- > > 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 > -- > 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 -- 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
