[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 04/12] aio: introduce aio_{disable, enable}_e
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [PATCH v5 04/12] aio: introduce aio_{disable, enable}_external |
Date: |
Wed, 21 Oct 2015 11:56:08 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Oct 21, 2015 at 10:06:41AM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> aio-posix.c | 3 ++-
> aio-win32.c | 3 ++-
> include/block/aio.h | 37 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/aio-posix.c b/aio-posix.c
> index f0f9122..0467f23 100644
> --- a/aio-posix.c
> +++ b/aio-posix.c
> @@ -261,7 +261,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
>
> /* fill pollfds */
> QLIST_FOREACH(node, &ctx->aio_handlers, node) {
> - if (!node->deleted && node->pfd.events) {
> + if (!node->deleted && node->pfd.events
> + && aio_node_check(ctx, node->is_external)) {
> add_pollfd(node);
> }
> }
> diff --git a/aio-win32.c b/aio-win32.c
> index 3110d85..43c4c79 100644
> --- a/aio-win32.c
> +++ b/aio-win32.c
> @@ -309,7 +309,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
> /* fill fd sets */
> count = 0;
> QLIST_FOREACH(node, &ctx->aio_handlers, node) {
> - if (!node->deleted && node->io_notify) {
> + if (!node->deleted && node->io_notify
> + && aio_node_check(ctx, node->is_external)) {
> events[count++] = event_notifier_get_handle(node->e);
> }
> }
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 12f1141..80151d1 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -122,6 +122,8 @@ struct AioContext {
>
> /* TimerLists for calling timers - one per clock type */
> QEMUTimerListGroup tlg;
> +
> + int external_disable_cnt;
> };
>
> /**
> @@ -375,4 +377,39 @@ static inline void aio_timer_init(AioContext *ctx,
> */
> int64_t aio_compute_timeout(AioContext *ctx);
>
> +/**
> + * aio_disable_external:
> + * @ctx: the aio context
> + *
> + * Disable the furthur processing of clients.
s/furthur/further
The comment below specifically references external clients - I think
the comments for aio_disable_external / aio_enable_external should be
worded similarly, so there is no confusion.
> + */
> +static inline void aio_disable_external(AioContext *ctx)
> +{
> + atomic_inc(&ctx->external_disable_cnt);
> +}
> +
> +/**
> + * aio_enable_external:
> + * @ctx: the aio context
> + *
> + * Disable the processing of external clients.
Should this comment read "Enable" instead of "Disable"?
> + */
> +static inline void aio_enable_external(AioContext *ctx)
> +{
> + atomic_dec(&ctx->external_disable_cnt);
Should we assert(ctx->external_disable_cnt >= 0)?
Additional comment: the function names aio_enable_external() and
aio_disable_external() may be a bit misleading (particularly
aio_enable_external()). It doesn't do a blanket enable of external
aio (i.e., it does not just blindly do ctx->external_disable_cnt = 0).
Perhaps something like aio_external_disable_inc/dec()? (I'm not real
fond of that, either).
Just something for thought.
> +}
> +
> +/**
> + * aio_node_check:
> + * @ctx: the aio context
> + * @is_external: Whether or not the checked node is an external event source.
> + *
> + * Check if the node's is_external flag is okey to be polled by the ctx at
> this
s/okey/okay
> + * moment. True means green light.
> + */
> +static inline bool aio_node_check(AioContext *ctx, bool is_external)
> +{
> + return !is_external || !atomic_read(&ctx->external_disable_cnt);
> +}
> +
It seems a little odd to me to have this helper function take the
is_external bool field from the node as the argument - any reason to
do that, rather than pass in the AioHandler and have aio_node_check()
parse whatever fields it deems necessary from it?
> #endif
> --
> 2.4.3
>
>
- [Qemu-block] [PATCH v5 00/12] block: Protect nested event loop with bdrv_drained_begin and bdrv_drained_end, Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 02/12] nbd: Mark fd handlers client type as "external", Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 01/12] aio: Add "is_external" flag for event handlers, Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 03/12] dataplane: Mark host notifiers' client type as "external", Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 04/12] aio: introduce aio_{disable, enable}_external, Fam Zheng, 2015/10/20
- Re: [Qemu-block] [PATCH v5 04/12] aio: introduce aio_{disable, enable}_external,
Jeff Cody <=
- [Qemu-block] [PATCH v5 05/12] block: Introduce "drained begin/end" API, Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 06/12] block: Add "drained begin/end" for transactional external snapshot, Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 07/12] block: Add "drained begin/end" for transactional backup, Fam Zheng, 2015/10/20
- [Qemu-block] [PATCH v5 08/12] block: Add "drained begin/end" for transactional blockdev-backup, Fam Zheng, 2015/10/20