On Fri, 2011-10-28 at 19:37 +0100, Ian Jackson wrote:
> This lock will be used to protect data structures which will be hung
> off the libxl_ctx in subsequent patches.
>
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
> tools/libxl/libxl.c | 3 +++
> tools/libxl/libxl_internal.h | 16 ++++++++++++++++
> 2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 5d448af..a3c9807 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -40,6 +40,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
> xentoollog_logger * lg)
> {
> libxl_ctx *ctx;
> struct stat stat_buf;
> + const pthread_mutex_t mutex_value =
> PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
>
> if (version != LIBXL_VERSION)
> return ERROR_VERSION;
> @@ -53,6 +54,8 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
> xentoollog_logger * lg)
> memset(ctx, 0, sizeof(libxl_ctx));
> ctx->lg = lg;
>
> + memcpy(&ctx->lock, &mutex_value, sizeof(ctx->lock));
Is this subtly different to
ctx->lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
in some way I'm missing?
> +
> if ( stat(XENSTORE_PID_FILE, &stat_buf) != 0 ) {
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Is xenstore daemon
> running?\n"
> "failed to stat %s", XENSTORE_PID_FILE);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 6d9da2c..79a9de4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -23,6 +23,7 @@
> #include <stdarg.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <pthread.h>
>
> #include <xs.h>
> #include <xenctrl.h>
> @@ -95,6 +96,9 @@ struct libxl__ctx {
> xc_interface *xch;
> struct xs_handle *xsh;
>
> + pthread_mutex_t lock; /* protects data structures hanging off the ctx */
> + /* always use MUTEX_LOCK and MUTEX_UNLOCK to manipulate this */
MUTEX is something of an implementation detail (although I admit we are
unlikely to use anything else), perhaps CTX_(UN)LOCK?
Perhaps give the variable a less obvious name to help discourage direct
use?
> +
> /* for callers who reap children willy-nilly; caller must only
> * set this after libxl_init and before any other call - or
> * may leave them untouched */
> @@ -577,6 +581,18 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc
> *gc, const char *s);
> #define CTX libxl__gc_owner(gc)
>
>
> +#define MUTEX_LOCK do { \
> + int mutex_r = pthread_mutex_lock(&CTX->lock); \
> + assert(!mutex_r); \
This assert is to catch EINVAL ("the mutex has not been properly
initialized") rather than EDEADLK ("the mutex is already locked by the
calling thread") since we asked for a non-error-checking recursive lock?
Since it is OK to take this lock recursively then it might be as well to
say so explicitly?
This is the first lock in libxl so I guess there isn't much of a locking
hierarchy yet. Are there any particular considerations which a caller
must make wrt its own locking?
> + } while(0)
> +
> +#define MUTEX_UNLOCK do { \
> + int mutex_r = pthread_mutex_unlock(&CTX->lock); \
> + assert(!mutex_r); \
> + } while(0)
> +
> +
> +
> /*
> * Inserts "elm_new" into the sorted list "head".
> *
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home