Re: [PATCH v2] fiemap: Allow to specify range to fiemap

Nikolay Borisov
2017-08-11T05:38:21Z

On 11.08.2017 00:57, Eric Sandeen wrote:
> On 8/10/17 2:22 PM, Eric Sandeen wrote:
>> On 8/10/17 2:12 PM, Darrick J. Wong wrote:
>>> On Thu, Aug 10, 2017 at 07:46:35PM +0300, Nikolay Borisov wrote:
>>>>
>>>>
>>>> On 10.08.2017 19:13, Darrick J. Wong wrote:
>>>>> We don't decrement len each time through the ioctl loop? Doesn't that
>>>>> cause us to request and retrieve rows for ranges we don't want?
>>>>
>>>> Hm, you are right, I believe on every iteration we should do:
>>>>
>>>> len -= last_logical - start_offset
>>>
>>> start_offset doesn't change, right?
>>>
>>> I would think len -= last_logical - fiemap->fm_start.
>>>
>>> Or just calculate end_offset = start_offset + length and set
>>> fiemap->fm_length = end_offset - last_logical...
>>>
>>> ...though now that I look at that file even closer I wonder what is
>>> going on with that outer while loop counting extents? Does -n exist to
>>> let users cap the number of records to print, even if that means we
>>> don't make it to the end of the range requested?
>>
>> -n should simply be the size of each fiemap request, i.e. how many to
>> request and fill on each iteration. It should not affect how many
>> get printed in the end.
>>
>> At least that's how xfs_bmap is supposed to work, and therefore in theory
>> how xfs_io's fiemap should work.
>
> ... except apparently that is not actually how it works :(

In my tests passing -n 1 doesn't print anything irrespective of whether
I use the newly added args, it was strange. It's supposed to define the
batch of extents to query at a time. However, that's definitely not what
it's doing... Also the man pages states that if -n is not passed fiemap
would query the number of extents required and use that, however the
code currently just uses the default of 32 extents ...
>
> -Eric
>