From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp1040.oracle.com ([156.151.31.81]:33666 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751970AbdHDUxz (ORCPT ); Fri, 4 Aug 2017 16:53:55 -0400 Date: Fri, 4 Aug 2017 13:53:38 -0700 From: "Darrick J. Wong" Subject: Re: [PATCH v2 5/5] xfs: toggle XFS_DIFLAG2_IOMAP_IMMUTABLE in response to fallocate Message-ID: <20170804205337.GG24087@magnolia> References: <150181368442.32119.13336247800141074356.stgit@dwillia2-desk3.amr.corp.intel.com> <150181371519.32119.2762945986660101008.stgit@dwillia2-desk3.amr.corp.intel.com> <20170804201412.GE24087@magnolia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Dan Williams Cc: Jan Kara , "linux-nvdimm@lists.01.org" , Dave Chinner , "linux-kernel@vger.kernel.org" , linux-xfs@vger.kernel.org, Jeff Moyer , Andy Lutomirski , linux-fsdevel , Ross Zwisler , Christoph Hellwig On Fri, Aug 04, 2017 at 01:47:32PM -0700, Dan Williams wrote: > On Fri, Aug 4, 2017 at 1:14 PM, Darrick J. Wong wrote: > > On Thu, Aug 03, 2017 at 07:28:35PM -0700, Dan Williams wrote: > >> After validating the state of the file as not having holes, shared > >> extents, or active mappings try to commit the > >> XFS_DIFLAG2_IOMAP_IMMUTABLE flag to the on-disk inode metadata. If that > >> succeeds then allow the S_IOMAP_IMMUTABLE to be set on the vfs inode. > >> > >> Cc: Jan Kara > >> Cc: Jeff Moyer > >> Cc: Christoph Hellwig > >> Cc: Ross Zwisler > >> Suggested-by: Dave Chinner > >> Suggested-by: "Darrick J. Wong" > >> Signed-off-by: Dan Williams > >> --- > >> fs/xfs/xfs_bmap_util.c | 32 ++++++++++++++++++++++++++++++++ > >> 1 file changed, 32 insertions(+) > >> > >> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > >> index 70ac2d33ab27..8464c25a2403 100644 > >> --- a/fs/xfs/xfs_bmap_util.c > >> +++ b/fs/xfs/xfs_bmap_util.c > >> @@ -1436,9 +1436,11 @@ xfs_seal_file_space( > >> xfs_off_t offset, > >> xfs_off_t len) > >> { > >> + struct xfs_mount *mp = ip->i_mount; > >> struct inode *inode = VFS_I(ip); > >> struct address_space *mapping = inode->i_mapping; > >> int error; > >> + struct xfs_trans *tp; > >> > >> ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL)); > >> > >> @@ -1454,6 +1456,10 @@ xfs_seal_file_space( > >> if (error) > >> return error; > >> > >> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); > >> + if (error) > >> + return error; > >> + > >> xfs_ilock(ip, XFS_ILOCK_EXCL); > >> /* > >> * Either the size changed after we performed allocation / > >> @@ -1486,10 +1492,20 @@ xfs_seal_file_space( > >> if (error < 0) > >> goto out_unlock; > >> > >> + xfs_trans_ijoin(tp, ip, 0); > > > > FWIW if you change that third parameter to XFS_ILOCK_EXCL then > > xfs_trans_commit will do the xfs_iunlock(ip, XFS_ILOCK_EXCL) for you if > > the commit succeeds... > > > >> + ip->i_d.di_flags2 |= XFS_DIFLAG2_IOMAP_IMMUTABLE; > >> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); > >> + error = xfs_trans_commit(tp); > >> + tp = NULL; /* nothing to cancel */ > >> + if (error) > >> + goto out_unlock; > >> + > >> inode->i_flags |= S_IOMAP_IMMUTABLE; > > > > ...and then you can just return out here. > > Do we not need to hold XFS_ILOCK_EXCL over ->i_flags changes, or is > XFS_IOLOCK_EXCL enough? Oh, heh, I missed a piece. Set the flag before the transaction commit, because if the commit fails the fs is shut down anyway. :) --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