Message ID | 1455645388-32401-3-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, 02/16 18:56, Paolo Bonzini wrote: > We want to remove throttled_reqs from block/io.c. This is the easy > part---hide the handling of throttled_reqs during disable/enable of > throttling within throttle-groups.c. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/io.c | 15 +-------------- > block/throttle-groups.c | 15 +++++++++++++++ > include/block/throttle-groups.h | 1 + > 3 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/block/io.c b/block/io.c > index e58cfe2..5ee5032 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -66,28 +66,15 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, > void bdrv_set_io_limits(BlockDriverState *bs, > ThrottleConfig *cfg) > { > - int i; > - > throttle_group_config(bs, cfg); > - > - for (i = 0; i < 2; i++) { > - qemu_co_enter_next(&bs->throttled_reqs[i]); > - } > } > > static void bdrv_start_throttled_reqs(BlockDriverState *bs) > { > bool enabled = bs->io_limits_enabled; > - int i; > > bs->io_limits_enabled = false; > - > - for (i = 0; i < 2; i++) { > - while (qemu_co_enter_next(&bs->throttled_reqs[i])) { > - ; > - } > - } > - > + throttle_group_restart_bs(bs); > bs->io_limits_enabled = enabled; > } > > diff --git a/block/throttle-groups.c b/block/throttle-groups.c > index 4920e09..eccfc0d 100644 > --- a/block/throttle-groups.c > +++ b/block/throttle-groups.c > @@ -313,6 +313,17 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, > qemu_mutex_unlock(&tg->lock); > } > > +void throttle_group_restart_bs(BlockDriverState *bs) > +{ > + int i; > + > + for (i = 0; i < 2; i++) { > + while (qemu_co_enter_next(&bs->throttled_reqs[i])) { > + ; > + } > + } > +} > + > /* Update the throttle configuration for a particular group. Similar > * to throttle_config(), but guarantees atomicity within the > * throttling group. > @@ -335,6 +346,10 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) > } > throttle_config(ts, tt, cfg); > qemu_mutex_unlock(&tg->lock); > + > + aio_context_acquire(bdrv_get_aio_context(bs)); > + throttle_group_restart_bs(bs); > + aio_context_release(bdrv_get_aio_context(bs)); Could you explain why does this hunk belong to this patch? Otherwise looks good. Fam > } > > /* Get the throttle configuration from a particular group. Similar to > diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h > index aba28f3..395f72d 100644 > --- a/include/block/throttle-groups.h > +++ b/include/block/throttle-groups.h > @@ -38,6 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg); > > void throttle_group_register_bs(BlockDriverState *bs, const char *groupname); > void throttle_group_unregister_bs(BlockDriverState *bs); > +void throttle_group_restart_bs(BlockDriverState *bs); > > void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, > unsigned int bytes, > -- > 2.5.0 > > >
On 09/03/2016 02:26, Fam Zheng wrote: >> diff --git a/block/throttle-groups.c b/block/throttle-groups.c >> index 4920e09..eccfc0d 100644 >> --- a/block/throttle-groups.c >> +++ b/block/throttle-groups.c >> @@ -313,6 +313,17 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, >> qemu_mutex_unlock(&tg->lock); >> } >> >> +void throttle_group_restart_bs(BlockDriverState *bs) >> +{ >> + int i; >> + >> + for (i = 0; i < 2; i++) { >> + while (qemu_co_enter_next(&bs->throttled_reqs[i])) { >> + ; >> + } >> + } >> +} >> + >> /* Update the throttle configuration for a particular group. Similar >> * to throttle_config(), but guarantees atomicity within the >> * throttling group. >> @@ -335,6 +346,10 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) >> } >> throttle_config(ts, tt, cfg); >> qemu_mutex_unlock(&tg->lock); >> + >> + aio_context_acquire(bdrv_get_aio_context(bs)); >> + throttle_group_restart_bs(bs); >> + aio_context_release(bdrv_get_aio_context(bs)); > > Could you explain why does this hunk belong to this patch? > > Otherwise looks good. Sure. It's moved from bdrv_set_io_limits, which calls throttle_group_config, to throttle_group_config itself: >> void bdrv_set_io_limits(BlockDriverState *bs, >> ThrottleConfig *cfg) >> { >> - int i; >> - >> throttle_group_config(bs, cfg); >> - >> - for (i = 0; i < 2; i++) { >> - qemu_co_enter_next(&bs->throttled_reqs[i]); >> - } >> } >> But in v2 I'll change it to only restart the first request so there is no semantic change. Paolo
diff --git a/block/io.c b/block/io.c index e58cfe2..5ee5032 100644 --- a/block/io.c +++ b/block/io.c @@ -66,28 +66,15 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, void bdrv_set_io_limits(BlockDriverState *bs, ThrottleConfig *cfg) { - int i; - throttle_group_config(bs, cfg); - - for (i = 0; i < 2; i++) { - qemu_co_enter_next(&bs->throttled_reqs[i]); - } } static void bdrv_start_throttled_reqs(BlockDriverState *bs) { bool enabled = bs->io_limits_enabled; - int i; bs->io_limits_enabled = false; - - for (i = 0; i < 2; i++) { - while (qemu_co_enter_next(&bs->throttled_reqs[i])) { - ; - } - } - + throttle_group_restart_bs(bs); bs->io_limits_enabled = enabled; } diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 4920e09..eccfc0d 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -313,6 +313,17 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, qemu_mutex_unlock(&tg->lock); } +void throttle_group_restart_bs(BlockDriverState *bs) +{ + int i; + + for (i = 0; i < 2; i++) { + while (qemu_co_enter_next(&bs->throttled_reqs[i])) { + ; + } + } +} + /* Update the throttle configuration for a particular group. Similar * to throttle_config(), but guarantees atomicity within the * throttling group. @@ -335,6 +346,10 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) } throttle_config(ts, tt, cfg); qemu_mutex_unlock(&tg->lock); + + aio_context_acquire(bdrv_get_aio_context(bs)); + throttle_group_restart_bs(bs); + aio_context_release(bdrv_get_aio_context(bs)); } /* Get the throttle configuration from a particular group. Similar to diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index aba28f3..395f72d 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -38,6 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg); void throttle_group_register_bs(BlockDriverState *bs, const char *groupname); void throttle_group_unregister_bs(BlockDriverState *bs); +void throttle_group_restart_bs(BlockDriverState *bs); void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, unsigned int bytes,
We want to remove throttled_reqs from block/io.c. This is the easy part---hide the handling of throttled_reqs during disable/enable of throttling within throttle-groups.c. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block/io.c | 15 +-------------- block/throttle-groups.c | 15 +++++++++++++++ include/block/throttle-groups.h | 1 + 3 files changed, 17 insertions(+), 14 deletions(-)