[0/2] file-posix: Cache next hole

Message

Max Reitz Feb. 11, 2021, 5:22 p.m. UTC

Hi,

On Launchpad, Alexandre has reported a performance problem with
SEEK_HOLE on filesystems with much fragmentation:

  https://bugs.launchpad.net/qemu/+bug/1912224

This isn’t the first report we’ve received on SEEK_HOLE being slow, and
we’ve already taken measures to address this issue.  For example,
block-status has a @want_zero parameter so that we won’t do such a seek
operation unless the caller needs that information.  qcow2 tries to
avoid falling through to the protocol level, because most of the time,
it itself knows well which clusters are zero and which aren’t.

But both of these measures don’t work e.g. when mirroring a raw file,
when we want to look up zero areas (because it may speed up the mirror
and save space), and where we don’t have a format layer that has
randomly accessible zero information.

Alexandre proposed caching the offset of the next hole, which we can do
in file-posix (unless the WRITE permission is shared), and which works
best for images that are (nearly) fully allocated.  Such images are also
the ones that profit the least off of the time lost on SEEK_HOLE
operations, i.e. where it’s most important to keep that time low.

I understand that caching filesystem information in file-posix is a bit
weird, but I’d like to hear what you think.  Alexandre has reported that
it alleviated his problem quite significantly (see comment 8 in the LP
report).


(Speaking of “unless the WRITE permission is shared”: mirror_top is a
bit broken in that it takes no permissions (but WRITE if necessary) and
shares everything.  That seems wrong.  Patch 1 addresses that, so that
patch 2 can actually do something when mirroring an image.)


Max Reitz (2):
  block/mirror: Fix mirror_top's permissions
  file-posix: Cache next hole

 block/file-posix.c | 81 +++++++++++++++++++++++++++++++++++++++++++++-
 block/mirror.c     | 32 ++++++++++++++----
 2 files changed, 105 insertions(+), 8 deletions(-)

Comments

Max Reitz March 29, 2021, 4:21 p.m. UTC | #1

On 11.02.21 18:22, Max Reitz wrote:
> Hi,

[...]

> (Speaking of “unless the WRITE permission is shared”: mirror_top is a
> bit broken in that it takes no permissions (but WRITE if necessary) and
> shares everything.  That seems wrong.  Patch 1 addresses that, so that
> patch 2 can actually do something when mirroring an image.)

I plan to send a v2 of patch 2 at some point, but for now I’ve applied 
patch 1 to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

https://bugzilla.redhat.com/show_bug.cgi?id=1940118 reports an abort, 
which I think can be avoided with patch 1 of this series: The mirror job 
lifts all permissions on the source node, so you can freely take locks 
with some other process, and then mirror_exit_common() fails when it 
tries to take those permissions back (at least when cancelling the job).

I plan to send an iotest for this, but getting this into rc1 is more 
important than waiting for the test, I think.

Max