| Message ID | 150181371519.32119.2762945986660101008.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <jack@suse.cz> > Cc: Jeff Moyer <jmoyer@redhat.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Suggested-by: Dave Chinner <david@fromorbit.com> > Suggested-by: "Darrick J. Wong" <darrick.wong@oracle.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > 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. --D > out_unlock: > xfs_iunlock(ip, XFS_ILOCK_EXCL); > + if (tp) > + xfs_trans_cancel(tp); > > return error; > } > @@ -1500,15 +1516,21 @@ xfs_unseal_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)); > > if (offset) > return -EINVAL; > > + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); > + if (error) > + return error; > + > xfs_ilock(ip, XFS_ILOCK_EXCL); > /* > * It does not make sense to unseal less than the full range of > @@ -1527,11 +1549,21 @@ xfs_unseal_file_space( > if (mapping_mapped(mapping)) > goto out_unlock; > > + xfs_trans_ijoin(tp, ip, 0); > + 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; > error = 0; > > out_unlock: > xfs_iunlock(ip, XFS_ILOCK_EXCL); > + if (tp) > + xfs_trans_cancel(tp); > > return error; > } > > -- > 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 Fri, Aug 4, 2017 at 1:14 PM, Darrick J. Wong <darrick.wong@oracle.com> 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 <jack@suse.cz> >> Cc: Jeff Moyer <jmoyer@redhat.com> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> >> Suggested-by: Dave Chinner <david@fromorbit.com> >> Suggested-by: "Darrick J. Wong" <darrick.wong@oracle.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> 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?
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 <darrick.wong@oracle.com> 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 <jack@suse.cz> > >> Cc: Jeff Moyer <jmoyer@redhat.com> > >> Cc: Christoph Hellwig <hch@lst.de> > >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > >> Suggested-by: Dave Chinner <david@fromorbit.com> > >> Suggested-by: "Darrick J. Wong" <darrick.wong@oracle.com> > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> > >> --- > >> 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
On Fri, Aug 4, 2017 at 1:53 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > 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 <darrick.wong@oracle.com> 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 <jack@suse.cz> >> >> Cc: Jeff Moyer <jmoyer@redhat.com> >> >> Cc: Christoph Hellwig <hch@lst.de> >> >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> >> >> Suggested-by: Dave Chinner <david@fromorbit.com> >> >> Suggested-by: "Darrick J. Wong" <darrick.wong@oracle.com> >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> >> --- >> >> 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. :) Ah, got it, thanks.
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); + 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; out_unlock: xfs_iunlock(ip, XFS_ILOCK_EXCL); + if (tp) + xfs_trans_cancel(tp); return error; } @@ -1500,15 +1516,21 @@ xfs_unseal_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)); if (offset) return -EINVAL; + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); + if (error) + return error; + xfs_ilock(ip, XFS_ILOCK_EXCL); /* * It does not make sense to unseal less than the full range of @@ -1527,11 +1549,21 @@ xfs_unseal_file_space( if (mapping_mapped(mapping)) goto out_unlock; + xfs_trans_ijoin(tp, ip, 0); + 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; error = 0; out_unlock: xfs_iunlock(ip, XFS_ILOCK_EXCL); + if (tp) + xfs_trans_cancel(tp); return error; }
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 <jack@suse.cz> Cc: Jeff Moyer <jmoyer@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> Suggested-by: Dave Chinner <david@fromorbit.com> Suggested-by: "Darrick J. Wong" <darrick.wong@oracle.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- fs/xfs/xfs_bmap_util.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)