linux-kernel - Re: [PATCH] xfs: Make fiemap works with sparse file.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C12C8BB.50604@oracle.com>
Date:	Sat, 12 Jun 2010 07:37:31 +0800
From:	Tao Ma <tao.ma@...cle.com>
To:	Eric Sandeen <sandeen@...deen.net>
CC:	xfs@....sgi.com, Christoph Hellwig <hch@....de>,
	linux-kernel@...r.kernel.org, Alex Elder <aelder@....com>
Subject: Re: [PATCH] xfs: Make fiemap works with sparse file.

Eric Sandeen wrote:
> Tao Ma wrote:
>   
>> In xfs_vn_fiemap, we set bvm_count to fi_extent_max + 1 and want
>> to return fi_extent_max extents, but actually it won't work for
>> a sparse file. The reason is that in xfs_getbmap we will
>> calculate holes and set it in 'out', while out is malloced by
>> bmv_count(fi_extent_max+1) which didn't consider holes. So in the
>> worst case, if 'out' vector looks like
>> [hole, extent, hole, extent, hole, ... hole, extent, hole],
>> we will only return half of fi_extent_max extents.
>>
>> So in xfs_vn_fiemap, we should consider this worst case. If the
>> user wants fi_extent_max extents, we need a 'out' with size of
>> 2 *fi_extent_max + 1.
>>     
>
> This all seems right to me, though your commit message above (+1)
> doesn't match the comment and code in the patch (+2)
>   
oh, yes, I will change the commit log and send a v2.
and can I add your ack for this patch?

Regards,
Tao
> -Eric
>
>   
>> Cc: Alex Elder <aelder@....com>
>> Cc: Christoph Hellwig <hch@....de>
>> Cc: Dave Chinner <david@...morbit.com>
>> Signed-off-by: Tao Ma <tao.ma@...cle.com>
>> ---
>>  fs/xfs/linux-2.6/xfs_iops.c |   16 ++++++++++++++--
>>  1 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/xfs/linux-2.6/xfs_iops.c b/fs/xfs/linux-2.6/xfs_iops.c
>> index 9c8019c..1db92e3 100644
>> --- a/fs/xfs/linux-2.6/xfs_iops.c
>> +++ b/fs/xfs/linux-2.6/xfs_iops.c
>> @@ -672,9 +672,21 @@ xfs_vn_fiemap(
>>  	else
>>  		bm.bmv_length = BTOBB(length);
>>  
>> -	/* We add one because in getbmap world count includes the header */
>> +	/*
>> +	 * It is a bit tricky for us to calculate the bmv_count from
>> +	 * fi_extent_max.
>> +	 * If we support to return fi_extent_max extents to the user,
>> +	 * we need at most 2 * fi_extent_max + 1 for bmv_count since
>> +	 * in xfs_getbmap we will calculate holes while fi_extent_max
>> +	 * don't have them. So in the worst case, bmv can looks like
>> +	 * [hole, extent, hole, extent, hole, ... hole, extent, hole].
>> +	 * So there will be 2 *fi_extent_max + 1.
>> +	 * What's more, in getbmap world count have to include the
>> +	 * header, so we need another bmv. So the total number will
>> +	 * be 2 * fieinfo->fi_extents_max + 2.
>> +	 */
>>  	bm.bmv_count = !fieinfo->fi_extents_max ? MAXEXTNUM :
>> -					fieinfo->fi_extents_max + 1;
>> +			2 * fieinfo->fi_extents_max + 2;
>>  	bm.bmv_count = min_t(__s32, bm.bmv_count,
>>  			     (PAGE_SIZE * 16 / sizeof(struct getbmapx)));
>>  	bm.bmv_iflags = BMV_IF_PREALLOC;
>>     
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/