Re: [Xen-devel] [XEN PATCH for-4.13 v3 6/7] libxl: Introduce libxl__ev_immediate

From: Ian Jackson <ian.jackson@citrix.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [XEN PATCH for-4.13 v3 6/7] libxl: Introduce libxl__ev_immediate
Date: Mon, 18 Nov 2019 17:28:17 +0000	[thread overview]
Message-ID: <24018.54449.703124.372517@mariner.uk.xensource.com> (raw)
In-Reply-To: <20191118171309.1459302-7-anthony.perard@citrix.com>

Anthony PERARD writes ("[XEN PATCH for-4.13 v3 6/7] libxl: Introduce libxl__ev_immediate"):
> This new ev allows to arrange a non-reentrant callback to be called.
> This happen immediately after the current event is processed and after
> other ev_immediates that would have already been registered.

Thanks for doing this work.

> +    LIBXL_TAILQ_FOREACH_SAFE(ei, &egc->ev_immediates, entry, ei_tmp) {
> +        LIBXL_TAILQ_REMOVE(&egc->ev_immediates, ei, entry);

I think LIBXL_TAILQ_FOREACH_SAFE is not safe enough here.
ei->callback might *add* things to egc->ev_immediates.  The manpage
just says

     However, unlike their unsafe counterparts, TAILQ_FOREACH and
     TAILQ_FOREACH_REVERSE permit to both remove var as well as free
     it from within the loop safely without interfering with the
     traversal.

I can't find an explicit statement about the allowable changes with
LIBXL_TAILQ_FOREACH but I expect they are "none".  See the loop in
ao__abort for what I think is the correct pattern (albeit embedded in
something more complex).

The rest of this LGTM.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel