Re: [PATCH RFC 3/8] xfs: convert inode from extents to local format

[Shan Hai <shan.hai@oracle.com>]

On 2018年07月06日 11:47, Darrick J. Wong wrote:
> On Fri, Jul 06, 2018 at 11:12:24AM +0800, Shan Hai wrote:
>> Introduce a function to convert an inode from extents to local
>> format. The conversion happens at writeback time, this avoids
>> interfering with the delayed allocation and page cache vs
>> xfs_buf operations, the will be inlined data is directly got
>> from the page which was read and modified by the iomap write actor.
>>
>> Signed-off-by: Shan Hai <shan.hai@oracle.com>
>> ---
>>   fs/xfs/xfs_aops.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
>> index 8eb3ba3d4d00..ac5a7695f363 100644
>> --- a/fs/xfs/xfs_aops.c
>> +++ b/fs/xfs/xfs_aops.c
>> @@ -9,6 +9,7 @@
>>   #include "xfs_log_format.h"
>>   #include "xfs_trans_resv.h"
>>   #include "xfs_mount.h"
>> +#include "xfs_defer.h"
>>   #include "xfs_inode.h"
>>   #include "xfs_trans.h"
>>   #include "xfs_inode_item.h"
>> @@ -887,6 +888,55 @@ xfs_map_cow(
>>   	return 0;
>>   }
>>   
>> +static int
>> +xfs_inode_extents_to_local(
>> +	xfs_inode_t		*ip,
>> +	int			whichfork,
>> +	struct page		*page)
>> +{
>> +	struct xfs_mount	*mp = ip->i_mount;
>> +	struct xfs_trans	*tp;
>> +	struct xfs_defer_ops	dfops;
>> +	xfs_fsblock_t		first_block;
>> +	int			logflags;
>> +	int			error = 0;
>> +
>> +	if (i_size_read(VFS_I(ip)) > XFS_IFORK_DSIZE(ip))
>> +		return 0;
> If it's too big to fit in local format, then isn't this an error?

No, it's not, because the written data travels to writeback means that 
the inode is
still in extents format since the higher level write_iter ends the write 
process by
copying the data to the data fork when the inode is in local format, it 
never let the
local format inode to go down the writeback path.

The xfs_writepage_map will continue as if nothing happens when 
xfs_inode_extents_to_local
returns 0.

Thanks
Shan Hai
> --D
>
>> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
>> +	if (error)
>> +		return error;
>> +
>> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
>> +	xfs_trans_ijoin(tp, ip, 0);
>> +
>> +	xfs_defer_init(&dfops, &first_block);
>> +
>> +	logflags = 0;
>> +	error = xfs_bmap_extents_to_local(tp, ip, &dfops, &logflags,
>> +			XFS_DATA_FORK, page);
>> +	if (error)
>> +		goto trans_cancel;
>> +
>> +	xfs_trans_log_inode(tp, ip, logflags);
>> +	error = xfs_defer_finish(&tp, &dfops);
>> +	if (error)
>> +		goto trans_cancel;
>> +	error = xfs_trans_commit(tp);
>> +	if (error)
>> +		goto error0;
>> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>> +
>> +	return 0;
>> +
>> +trans_cancel:
>> +	xfs_defer_cancel(&dfops);
>> +	xfs_trans_cancel(tp);
>> +error0:
>> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>> +	return error;
>> +}
>>   /*
>>    * We implement an immediate ioend submission policy here to avoid needing to
>>    * chain multiple ioends and hence nest mempool allocations which can violate
>> @@ -920,6 +970,8 @@ xfs_writepage_map(
>>   	int			count = 0;
>>   	int			uptodate = 1;
>>   	unsigned int		new_type;
>> +	xfs_inode_t		*ip = XFS_I(inode);
>> +	struct xfs_mount	*mp = ip->i_mount;
>>   
>>   	bh = head = page_buffers(page);
>>   	offset = page_offset(page);
>> @@ -1044,6 +1096,14 @@ xfs_writepage_map(
>>   		end_page_writeback(page);
>>   	}
>>   
>> +	if (!error) {
>> +		if (xfs_sb_version_hasinlinedata(&mp->m_sb) &&
>> +				i_size_read(inode) <= XFS_IFORK_DSIZE(ip)) {
>> +			error = xfs_inode_extents_to_local(ip, XFS_DATA_FORK,
>> +								page);
>> +		}
>> +	}
>> +
>>   	mapping_set_error(page->mapping, error);
>>   	return error;
>>   }
>> -- 
>> 2.11.0
>>
>> --
>> 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