Re: [RFC PATCH 1/8] fat: support FIEMAP

[OGAWA Hirofumi]

Matheus Alves de Almeida <matheus.aalmeida@xxxxxxxxxxxx> writes:


[...]

>>>  	if (fat_cache_lookup(inode, cluster, &cid, fclus, dclus) < 0) {
>>> -		/*
>>> -		 * dummy, always not contiguous
>>> -		 * This is reinitialized by cache_init(), later.
>>> -		 */
>>> -		cache_init(&cid, -1, -1);
>>> +		/* Cache the contiguous prefix from the known first cluster. */
>>> +		cache_init(&cid, 0, *dclus);
>>
>>Doesn't this add a needless cache? 0 is cached in the inode itself,
>>isn't it? (and can be waste small fatent cache slots?)
>>
>
> This is mostly for caching the number of contiguous clusters starting
> from 0. i_start only really tells us the first cluster of the file. I'm
> not exactly sure of the best way to approach this, though. I guess we
> can skip caching when its a one cluster range in this case?
>
> What do you think?

Ah, sure. I think because it is fat_cache_lookup() and stuff are
implemented for nr_cont == 1. So I think better way would be supporting
the range lookup in whole cache stuff. (however, it can be relatively
complex)

Thanks.

>>>  	}
>>>  
>>> +	fcluster = *fclus;
>>> +	dcluster = *dclus;
>>>  	fatent_init(&fatent);
>>> -	while (*fclus < cluster) {
>>> +	while (fcluster < last) {
>>> +		if (nr_cont && fcluster == cluster && cid.fcluster != -1) {
>>
>>cid.fcluster == -1 is possible with this patch?
>>
>
> Good catch.
>
>>> +			fcluster = min(last, cid.fcluster + cid.nr_contig);
>>> +			dcluster = cid.dcluster + fcluster - cid.fcluster;
>>> +			if (fcluster == last)
>>> +				break;
>>> +		}
>>> +
>
> Thanks,
> Matheus

-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>