Re: [PATCH] Btrfs: use larger limit for transition of logical to inode

[Liu Bo <bo.li.liu@oracle.com>]

On 08/23/2012 06:07 PM, Jan Schmidt wrote:
> On Thu, August 23, 2012 at 10:56 (+0200), Liu Bo wrote:
>> This is the change of the kernel side.
>>
>> Transition of logical to inode used to have a limit 4096 on inode container's
>> size, but the limit is not large enough for a data with a great many of refs,
>> so when resolving logical address, we can end up with
>> "ioctl ret=0, bytes_left=0, bytes_missing=19944, cnt=510, missed=2493"
>>
>> This changes to regard 4096 as the lowest limit.
>>
>> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
>> ---
>>  fs/btrfs/ioctl.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 9449b84..525915f 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3232,7 +3232,7 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_root *root,
>>  		goto out;
>>  	}
>>  
>> -	size = min_t(u32, loi->size, 4096);
>> +	size = max_t(u32, loi->size, 4096);
> 
> Hum. I added this because I wanted to avoid allocations > PAGE_SIZE. We're doing
> kmalloc GFP_NOFS with whatever one enters as size, I'm not sure that's a good
> idea without any sanitizing.
> 

Yeah, I agree.

So we do need to make some sanity checks, according to my tests,
we need about 30k to resolve a file shared by 4000 snapshots.

What about 32k as a upside limit?

> Second, we should probably add a fall back option to vmalloc, in case kmalloc
> fails? Or should we even go for vmalloc directly, what do you think?
>

Given loi->size is not reliable, going for vmalloc for an ioctl is reasonable.

thanks,
liubo

> Thanks,
> -Jan
> 
>>  	inodes = init_data_container(size);
>>  	if (IS_ERR(inodes)) {
>>  		ret = PTR_ERR(inodes);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>