Re: [Qemu-devel] [Qemu-block] Once again with feeling: work-around for slow SEEK_HOLE on Linux tmpfs.

[Eric Blake]

On 11/16/2016 01:48 PM, Christopher Oliver wrote:
> 
> 

Attaching the patch (rather than including it inline) requires reviewers
to save the attachment off to a file in order to even see what you
wrote.  To save others some time, I'm pasting the text and replying inline:

> The following patch is a work-around for slow SEEK_HOLE on some filesystems.

The subject line is too long and doesn't match the usual pattern of
"category: short synopsis".

> Specifically, SEEK_HOLE on a dense file on Linux tmpfs is linear time in
> the length.  This slows qemu-img to a crawl as it runs SEEK_DATA/SEEK_HOLE
> pairs over the length of the image it's reading stepping by small deltas.
> 
> The key observation is that if the descriptor is read-only, and there are
> no writers anywhere else (that's undefined behavior anyhow, right?), then a
> hole seek in the interval from the previous start to the previously found
> hole will find the same hole.
> 
> Signed-off-by: Christopher Oliver

The S-o-b is incorrect; it is missing an email address.  Without that,
the patch cannot be accepted.  You'll want to try again, but this time,
I suggest getting 'qemu send-email' working to the point that you can
send yourself an inline patch (not an attachment), before sending v2 to
the list; you may also want to check out other patch submission
guidelines here:

http://wiki.qemu.org/Contribute/SubmitAPatch

> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 28b47d9..b45defe 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -136,6 +136,8 @@ typedef struct BDRVRawState {
>      int type;
>      int open_flags;
>      size_t buf_align;
> +    off_t last_hole;
> +    off_t hole_follows;
>  
>  #ifdef CONFIG_XFS
>      bool is_xfs:1;
> @@ -470,6 +472,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> *options,
>  
>      s->has_discard = true;
>      s->has_write_zeroes = true;
> +    s->last_hole = -1;
>      bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
>      if ((bs->open_flags & BDRV_O_NOCACHE) != 0) {
>          s->needs_alignment = true;
> @@ -1710,7 +1713,24 @@ static int find_allocation(BlockDriverState *bs, off_t 
> start,
>       * H4. offs < 0, errno != ENXIO: we learned nothing
>       *     Pretend we know nothing at all, i.e. "forget" about D1.
>       */
> -    offs = lseek(s->fd, start, SEEK_HOLE);
> +    /* Addendum: Since HOLE seeks are expensive on some filesystems

Can anything be done to Linux tmpfs to make HOLE seeks less expensive?

> +     * (e.g. tmpfs) and holes don't change when an image is read only,
> +     * cache the range from a start to a hold and return that value

s/hold/hole/

> +     * for requests in that interval.  Outside of that interval, seek
> +     * and cache the new range.
> +     */
> +    if  ((s->open_flags & (O_RDWR|O_RDONLY)) == O_RDONLY) {
> +        if (start <= s->last_hole && start >= s->hole_follows) {
> +            offs = lseek(s->fd, s->last_hole, SEEK_SET);
> +        } else {
> +            offs = lseek(s->fd, start, SEEK_HOLE);
> +            s->last_hole = offs;
> +            s->hole_follows = start;
> +        }
> +    } else {
> +        offs = lseek(s->fd, start, SEEK_HOLE);
> +    }
> +
>      if (offs < 0) {
>          return -errno;          /* D1 and (H3 or H4) */
>      }

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature