[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 5/5] block: Move request_alignment
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 5/5] block: Move request_alignment into BlockLimit |
Date: |
Tue, 14 Jun 2016 10:05:46 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 14.06.2016 um 06:39 hat Eric Blake geschrieben:
> On 06/07/2016 04:08 AM, Kevin Wolf wrote:
>
> >> Found it; squash this in (or use it as an argument why we don't want
> >> request_alignment in bs->bl after all):
> >
> > This hunk doesn't make sense to me. For the correctness of the code it
> > shouldn't make a difference whether the alignment happens before passing
> > the request to file/raw-posix or already in the raw format layer.
> >
> > The cause for the hang you're seeing is probably that the request is
> > already aligned before the blkdebug layer and therefore the blkdebug
> > events aren't generated any more. That's a problem with the test (I'm
> > considering the blkdebug events part of the test infrastructure),
> > however, and not with the code.
> >
>
> Yes, it's definitely a hang caused by the test expecting an unalignment
> event, but the inheritance chain now causes things to be aligned to
> begin with and nothing unaligned happens after all.
>
> > Kevin
> >
> >> diff --git i/block/raw_bsd.c w/block/raw_bsd.c
> >> index b1d5237..c3c2246 100644
> >> --- i/block/raw_bsd.c
> >> +++ w/block/raw_bsd.c
> >> @@ -152,7 +152,11 @@ static int raw_get_info(BlockDriverState *bs,
> >> BlockDriverInfo *bdi)
> >>
> >> static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
> >> {
> >> + /* Inherit all limits except for request_alignment */
> >> + int request_alignment = bs->bl.request_alignment;
> >> +
> >> bs->bl = bs->file->bs->bl;
> >> + bs->bl.request_alignment = request_alignment;
>
> Any ideas on how to fix the test, then? Have two blkdebug devices
> nested atop one another, since those are the devices where we can
> explicitly override alignment?
Interesting idea. Maybe that's a good option if it works.
> (normally, you'd _expect_ the chain to
> inherit the worst-case alignment of all BDS in the chain, so blkdebug is
> the way around it).
Actually, I think there are two cases.
The first one is that you get a request and want to know what to do with
it. Here you don't want to inherit the worst-case alignment. You'd
rather want to enforce alignment only when it is actually needed. For
example, if you have a cache=none backing file with 4k alignment, but a
cache=writeback overlay, and you don't actually access the backing file
with this request, it would be wasteful to align the request. You also
don't really know that a driver will issue a misaligned request (or any
request at all) to the lower layer just because it got called with one.
The second case is when you want to issue a request. For example, let's
take the qcow2 cache here, which has 64k cached and needs to update a
few bytes. Currently it always writes the full 64k (and with 1 MB
clusters a full megabyte), but what it really should do is consider the
worst-case alignment.
This is comparable to the difference between bdrv_opt_mem_align(), which
is used in qemu_blockalign() where we want to create a buffer that works
even in the worst case, and bdrv_min_mem_align(), which is used in
bdrv_qiov_is_aligned() in order to determine whether we must align an
existing request.
> That's the only thing left before I repost the series, so I may just
> post the last patch as RFC and play with it a bit more...
And in the light of the above, maybe the solution is as easy as changing
raw_refresh_limits() to set bs->bl.request_alignment = 1.
Kevin
pgpIySCYJtlFu.pgp
Description: PGP signature
- Re: [Qemu-block] [PATCH 3/5] block: Switch transfer length bounds to byte-based, (continued)
[Qemu-block] [PATCH 5/5] block: Move request_alignment into BlockLimit, Eric Blake, 2016/06/03
Re: [Qemu-block] [PATCH 5/5] block: Move request_alignment into BlockLimit, Kevin Wolf, 2016/06/07
[Qemu-block] [PATCH 4/5] block: Switch discard length bounds to byte-based, Eric Blake, 2016/06/03
[Qemu-block] [PATCH 6/5] block: Fix harmless off-by-one in bdrv_aligned_preadv(), Eric Blake, 2016/06/03
[Qemu-block] [PATCH 7/5] block: Refactor zero_beyond_eof hack in bdrv_aligned_preadv(), Eric Blake, 2016/06/03