Re: [Qemu-block] [PATCH] block: don't probe zeroes in bs->file by default on block_status

[Vladimir Sementsov-Ogievskiy]

11.01.2019 19:02, Eric Blake wrote:
> On 1/11/19 1:54 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>>
>>> How much performance can we buy back without any knobs at all, if we
>>> just taught posix-file.c to cache lseek() results?  That is, when
>>> visiting a file sequentially, if lseek(fd, 0, SEEK_HOLE) returns EOF on
>>> our first block status query, then all subsequent block status queries
>>> fall within what we know to be data, and we can skip the lseek() calls.
>>
>> EOF is bad mark I think. We may have a small hole not far from EOF, which
>> will lead to the same performance, but not EOF returned.
> 
> EOF was just an example for a file that has no holes. But even for a
> file with holes, caching should help.  That is, if I have a raw file with:
> 
> 1M data | 1M hole | 1M data | EOF
> 
> but a qcow2 file that was created in an out-of-order fashion, so that
> all clusters are discontiguous, then our current code may do something
> like the following sequence:
> 
> block_status 0 - maps to 64k of host file at offset 0x20000
>   - lseek(0x20000) detects that we are in a data portion, and the next
> hole begins at 0x100000
>   - but because the next cluster is not at 0x30000, we throw away the
> information for 0x30000 to 0x100000
> block_status 64k - maps to 64k of host file at offset 0x40000
>   - lseek(0x40000) detects that we are in a data portion, and the next
> hole begins at 0x100000
>   - but because the next cluster is not at 0x50000, we throw away the
> information for 0x50000 to 0x100000
> block status 128k - maps to 64k of host file at offset 0x30000
>   - lseek(0x30000) detects that we are in a data portion, and the next
> hole begins at 0x100000
> ...
> 
> Even a dumb most-recent use cache will speed this up: both the second
> and third queries above can be avoided because we know that both 0x40000
> and 0x30000 the second query at 0x40000 can be skipped (0x40000 is
> between our most recent lseek at 0x20000 and hole at 0x10000)

Is it correct just use results from previous iterations? In mirror source
is active and may change.

> 
> Make the cache slightly larger, or use a bitmap with 2 bits per cluster
> (tracking unknown, known-data, known-hole), with proper flushing of the
> cache as we write to the image, or whatever, and we should automatically
> get some performance improvements by using fewer lseek() anywhere that
> we remember what previous lseek() already told us, with no knobs needed.
> 

So the cache should consider all writes and discards. And it is obviously
more difficult to implement it, than just don't call this lseek. And I
don't understand, why cache + lseek is better for the case when we don't
need nor the lseek neither the cache. Is this all to not add an option?
Also Kevin objects to caching lseek in parallel sub-thread.

-- 
Best regards,
Vladimir