[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 06/20] block: make bdrv_co_do_write_zeroes st
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 06/20] block: make bdrv_co_do_write_zeroes stricter in producing aligned requests |
Date: |
Wed, 20 Nov 2013 15:29:22 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Nov 20, 2013 at 12:01:46PM +0100, Paolo Bonzini wrote:
> Il 20/11/2013 11:22, Stefan Hajnoczi ha scritto:
> >> > + && num > bs->bl.write_zeroes_alignment) {
> > Here '>' is used...
> >
> >> > + if (sector_num % bs->bl.write_zeroes_alignment != 0) {
> >> > + /* Make a small request up to the first aligned sector.
> >> > */
> >> > num = bs->bl.write_zeroes_alignment;
> >> > + num -= sector_num % bs->bl.write_zeroes_alignment;
> >> > + } else if (num >= bs->bl.write_zeroes_alignment) {
> > ...but here '>=' is used.
> >
> > The == case here cannot happen. Did you mean '>=' in both places?
>
> I meant what I wrote, in the sense that the two "if"s make sense the
> way I wrote them. However, it is not too clear indeed.
>
> > if (bs->bl.write_zeroes_alignment
> > && num > bs->bl.write_zeroes_alignment) {
>
> Here, '>' is a necessary (though not sufficient) for being able to
> split the operation in one misaligned request and one aligned request.
>
> If '<', the request is misaligned in either the starting sector
> or the ending sector (or both), and there's no need to split it.
>
> If '==', either the request is aligned or we can only split it
> in two parts but they remain misaligned. In either case there's
> no need to do anyting.
>
> Because the condition is not sufficient, we may end up splitting a
> request that cannot be aligned anyway (e.g. sector_num = 1, num = 129,
> alignment = 128; will be split into [1,128) and [128,130) which are
> both misaligned. This is not important.
>
> > if (sector_num % bs->bl.write_zeroes_alignment != 0) {
> > /* Make a small request up to the first aligned sector. */
> > num = bs->bl.write_zeroes_alignment;
> > num -= sector_num % bs->bl.write_zeroes_alignment;
> > } else if (num >= bs->bl.write_zeroes_alignment) {
> > /* Shorten the request to the last aligned sector. */
> > num -= (sector_num + num) % bs->bl.write_zeroes_alignment;
>
> Here, the "if" checks that we can have no underflow in the subtraction.
> However, in addition to what you pointed out, it's not immediately obvious
> that the subtraction has no effect if sector_num and num are correctly
> aligned.
>
> So I will rewrite the "if" this way:
>
> if (sector_num % bs->bl.write_zeroes_alignment != 0) {
> /* Make a small request up to the first aligned sector. */
> num = bs->bl.write_zeroes_alignment;
> num -= sector_num % bs->bl.write_zeroes_alignment;
> } else if ((sector_num + num) % bs->bl.write_zeroes_alignment !=
> 0) {
> /* Shorten the request to the last aligned sector. num cannot
> * underflow because num > bs->bl.write_zeroes_alignment.
> */
> num -= (sector_num + num) % bs->bl.write_zeroes_alignment;
> }
Thanks, that is clearer.
Stefan
- [Qemu-devel] [PATCH v2 02/20] block: add flags to BlockRequest, (continued)
- [Qemu-devel] [PATCH v2 02/20] block: add flags to BlockRequest, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 01/20] block: generalize BlockLimits handling to cover bdrv_aio_discard too, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 04/20] block: add bdrv_aio_write_zeroes, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 03/20] block: add flags argument to bdrv_co_write_zeroes tracepoint, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 05/20] block: handle ENOTSUP from discard in generic code, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 06/20] block: make bdrv_co_do_write_zeroes stricter in producing aligned requests, Paolo Bonzini, 2013/11/19
- Re: [Qemu-devel] [PATCH v2 06/20] block: make bdrv_co_do_write_zeroes stricter in producing aligned requests, Peter Lieven, 2013/11/21
- [Qemu-devel] [PATCH v2 07/20] vpc, vhdx: add get_info, Paolo Bonzini, 2013/11/19
- [Qemu-devel] [PATCH v2 08/20] block drivers: add discard/write_zeroes properties to bdrv_get_info implementation, Paolo Bonzini, 2013/11/19
[Qemu-devel] [PATCH v2 10/20] block/iscsi: remove .bdrv_has_zero_init, Paolo Bonzini, 2013/11/19