[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4himTRCu9t2zyyxHEDRKmNQ=h7z9aNxqdfEuxSM=A4bhg@mail.gmail.com>
Date: Fri, 4 Aug 2017 13:47:32 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: "Darrick J. Wong" <darrick.wong@...cle.com>
Cc: Jan Kara <jack@...e.cz>,
"linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
Dave Chinner <david@...morbit.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-xfs@...r.kernel.org, Jeff Moyer <jmoyer@...hat.com>,
Andy Lutomirski <luto@...nel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
Ross Zwisler <ross.zwisler@...ux.intel.com>,
Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v2 5/5] xfs: toggle XFS_DIFLAG2_IOMAP_IMMUTABLE in
response to fallocate
On Fri, Aug 4, 2017 at 1:14 PM, Darrick J. Wong <darrick.wong@...cle.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@...e.cz>
>> Cc: Jeff Moyer <jmoyer@...hat.com>
>> Cc: Christoph Hellwig <hch@....de>
>> Cc: Ross Zwisler <ross.zwisler@...ux.intel.com>
>> Suggested-by: Dave Chinner <david@...morbit.com>
>> Suggested-by: "Darrick J. Wong" <darrick.wong@...cle.com>
>> Signed-off-by: Dan Williams <dan.j.williams@...el.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?
Powered by blists - more mailing lists