qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 4/7] block: remove legacy I/O throttling


From: Manos Pitsidianakis
Subject: Re: [Qemu-block] [PATCH v3 4/7] block: remove legacy I/O throttling
Date: Fri, 8 Sep 2017 20:47:16 +0300
User-agent: NeoMutt/20170609-57-1e93be (1.8.3)

On Fri, Sep 08, 2017 at 06:00:11PM +0200, Kevin Wolf wrote:
Am 08.09.2017 um 17:44 hat Manos Pitsidianakis geschrieben:
On Thu, Sep 07, 2017 at 03:26:11PM +0200, Kevin Wolf wrote:
> We shouldn't really need any throttling code in
> blk_root_drained_begin/end any more now because the throttle node will
> be drained. If this code is necessary, a bdrv_drain() on an explicit
> throttle node will work differently from one on an implicit one.
>
> Unfortunately, this seems to be true about the throttle node. Implicit
> throttle nodes will keep ignoring the throttle limit in order to
> complete the drain request quickly, where as explicit throttle nodes
> will process their requests at the configured speed before the drain
> request can be completed.
>
> This doesn't feel right to me, both should behave the same.
>
> Kevin
>

I suppose we can implement bdrv_co_drain and increase io_limits_disabled
from inside the driver. And then remove the implicit filter logic from
blk_root_drained_begin. But there's no _end callback equivalent so we can't
decrease io_limits_disabled at the end of the drain. So I think there are
two options:

- make a bdrv_co_drain_end cb and recurse in blk_root_drain_end for all
children to call it. Old behavior of I/O bursts (?) during a drain is  kept.

This is the solution I was thinking of. It was always odd to have a
drained_begin/end pair in the external interface and in BdrvChildRole,
but not in BlockDriver. So it was to be expected that we'd need this
sooner or later.

- remove io_limits_disabled and let throttled requests obey limits  during a
drain

This was discussed earlier (at least when the disable code was
introduced in BlockBackend, but I think actually more than once), and
even though everyone agreed that ignoring the limits is ugly, we
seem to have come to the conclusion that it's the least bad option.
blk_drain() blocks and makes everything else hang, so we don't want it
to wait for several seconds.

Kevin

That makes sense. I will look into this and resubmit the series with this additional change.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]