Re: [PATCH 1/2] coroutine: New module

[Rusty Russell <rusty@rustcorp.com.au>]

David Gibson <david@gibson.dropbear.id.au> writes:
> + * This code has helper functions for implementing co-routines, that
> + * is, explicit co-operative context switching.  It's intended to
> + * provide similar functionality to 
> + *
> + * Author: David Gibson <david@gibson.dropbear.id.au>

I believe David Gibson provides more functionality than this module :)

> +bool coroutine_stack_valid(struct coroutine_stack *stack)
> +{
> +	return stack
> +		&& (stack->magic == COROUTINE_STACK_MAGIC)
> +		&& (stack->size >= COROUTINE_MIN_STKSZ);
> +}

My (loose) standard here has been as in ccan/list: a pass-through
function which aborts if abortstring is non-NULL:

        struct coroutine_stack *coroutine_stack_check(const struct coroutine_stack *stack, const char *abortstr);

Then in the header:

#ifdef CCAN_COROUTINE_DEBUG
#define coroutine_stack_debug(s, loc) coroutine_stack_check((s), loc)
#define coroutine_state_debug(s, loc) coroutine_state_check((s), loc)
#else
#define coroutine_stack_debug(s, loc) ((void)loc, s)
#define coroutine_state_debug(s, loc) ((void)loc, s)
#endif

And sprinkle those liberally through your code.  That means they get a
nice crash if CCAN_COROUTINE_DEBUG is defined, otherwise they can still
call coroutine_stack_check() themselves if they want.

> +#if HAVE_UCONTEXT
> +void coroutine_init(struct coroutine_state *cs,
> +		    void (*fn)(void *), void *arg,
> +		    struct coroutine_stack *stack)
> +{
> +	getcontext (&cs->uc);
> +
> +	coroutine_uc_stack(&cs->uc.uc_stack, stack);
> +
> +        if (HAVE_POINTER_SAFE_MAKECONTEXT) {
> +                makecontext(&cs->uc, (void *)fn, 1, arg);
> +        } else {
> +                ptrdiff_t si = ptr2int(arg);
> +                ptrdiff_t mask = (1UL << (sizeof(int) * 8)) - 1;
> +                int lo = si & mask;
> +                int hi = si >> (sizeof(int) * 8);
> +
> +                makecontext(&cs->uc, (void *)fn, 2, lo, hi);
> +        }
> +	
> +}
> +
> +void coroutine_jump(const struct coroutine_state *to)
> +{
> +	setcontext(&to->uc);
> +	assert(0);
> +}
> +
> +void coroutine_switch(struct coroutine_state *from,
> +		      const struct coroutine_state *to)
> +{
> +	int rc;
> +
> +	rc = swapcontext(&from->uc, &to->uc);
> +	assert(rc == 0);
> +}



> +#endif /* HAVE_UCONTEXT */
> diff --git a/ccan/coroutine/coroutine.h b/ccan/coroutine/coroutine.h
> new file mode 100644
> index 0000000..6eba4a3
> --- /dev/null
> +++ b/ccan/coroutine/coroutine.h
> @@ -0,0 +1,219 @@
> +/* Licensed under LGPLv2.1+ - see LICENSE file for details */
> +#ifndef CCAN_COROUTINE_H
> +#define CCAN_COROUTINE_H
> +#include "config.h"
> +
> +#include <stddef.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <assert.h>
> +
> +#include <ccan/compiler/compiler.h>
> +
> +/**
> + * struct coroutine_stack
> + *
> + * Describes a stack suitable for executing a coroutine.  This
> + * structure is always contained within the stack it describes.
> + */
> +struct coroutine_stack;
> +
> +/**
> + * struct coroutine_state
> + *
> + * Describes the state of an in-progress coroutine.
> + */
> +struct coroutine_state;
> +
> +/*
> + * Stack management
> + */
> +
> +/**
> + * COROUTINE_STK_OVERHEAD - internal stack overhead
> + *
> + * Number of bytes of a stack which coroutine needs for its own
> + * tracking information.
> + */
> +#define COROUTINE_STK_OVERHEAD		(sizeof(uint64_t) + sizeof(size_t))
> +
> +/**
> + * COROUTINE_MIN_STKSZ - Minimum coroutine stack size
> + *
> + * Contains the minimum size for a coroutine stack (not including
> + * overhead).  On systems with MINSTKSZ, guaranteed to be at least as
> + * large as MINSTKSZ.
> + */
> +#define COROUTINE_MIN_STKSZ		2048
> +
> +/**
> + * COROUTINE_STACK_MAGIC - Magic number for coroutine stacks
> + */
> +#define COROUTINE_STACK_MAGIC		0xc040c040574c574c
> +
> +
> +/**
> + * coroutine_stack_init - Prepare a coroutine stack in an existing buffer
> + * @buf: buffer to use for the coroutine stack
> + * @bufsize: size of @buf
> + * @metasize: size of metadata to add to the stack (not including
> + *            coroutine internal overhead)
> + *
> + * Prepares @buf for use as a coroutine stack, returning a
> + * coroutine_stack *, allocated from within the buffer.  Returns NULL
> + * on failure.
> + *
> + * This will fail if the bufsize < (COROUTINE_MIN_STKSZ +
> + * COROUTINE_STK_OVERHEAD + metasize).
> + */
> +struct coroutine_stack *coroutine_stack_init(void *buf, size_t bufsize,
> +					     size_t metasize);
> +
> +/**
> + * coroutine_stack_init - Stop using a coroutine stack
> + * @stack: coroutine stack to release
> + *
> + * This releases @stack, making it no longer suitable for use as a
> + * coroutine stack.
> + */
> +void coroutine_stack_release(struct coroutine_stack *stack);
> +
> +/**
> + * coroutine_stack_valid - Determine if a coroutine stack looks valid
> + * @stack: stack to check
> + *
> + * Returns true if @stack appears to be a valid coroutine stack, false
> + * otherwise.
> + */
> +bool coroutine_stack_valid(struct coroutine_stack *stack);
> +
> +/**
> + * coroutine_stack_to_metadata - Returns pointer to user's metadata
> + *                               allocated within the stack
> + * @stack: coroutine stack
> + * @metasize: size of metadata
> + *
> + * Returns a pointer to the metadata area within @stack.  This is of
> + * size given at initialization time, and won't be overwritten by
> + * coroutines executing on the stack.  It's up to the caller what to
> + * put in here. @metasize must be equal to the value passed to
> + * coroutine_stack_init().
> + */
> +static inline void *coroutine_stack_to_metadata(struct coroutine_stack *stack,
> +						size_t metasize)
> +{
> +#if HAVE_STACK_GROWS_UPWARDS
> +	return (char *)stack - metasize;
> +#else
> +	return (char *)stack + COROUTINE_STK_OVERHEAD;
> +#endif
> +}
> +
> +/**
> + * coroutine_stack_from_metadata - Returns pointer to coroutine stack
> + *                                 pointer given pointer to user metadata
> + * @metadat: user metadata within a stack
> + * @metasize: size of metadata
> + *
> + * Returns a pointer to the coroutine_stack handle within a stack.
> + * The argument must be a pointer returned by
> + * coroutine_stack_to_metadata() at an earlier time. @metasize must be
> + * equal to the value passed to coroutine_stack_init().
> + */
> +static inline struct coroutine_stack *
> +coroutine_stack_from_metadata(void *metadata, size_t metasize)
> +{
> +#if HAVE_STACK_GROWS_UPWARDS
> +	return (struct coroutine_stack *)((char *)metadata + metasize);
> +#else
> +	return (struct coroutine_stack *)((char *)metadata
> +					  - COROUTINE_STK_OVERHEAD);
> +#endif
> +}
> +
> +/**
> + * coroutine_stack_size - Return size of a coroutine stack
> + * @stack: coroutine stack
> + *
> + * Returns the size of the coroutine stack @stack.  This does not
> + * include the overhead of struct coroutine_stack or metdata.
> + */
> +size_t coroutine_stack_size(const struct coroutine_stack *stack);
>
> +/**
> + * coroutine_init - Prepare a coroutine for execution
> + * @cs: coroutine_state structure to initialize
> + * @fn: function to start executing in the coroutine
> + * @arg: argument for @fn
> + * @stack: stack to use for the coroutine
> + *
> + * Prepares @cs as a new coroutine which will execute starting with
> + * function @fn, using stack @stack.
> + */
> +void coroutine_init(struct coroutine_state *cs,
> +		    void (*fn)(void *), void *arg,
> +		    struct coroutine_stack *stack);

Minor suggestion, typesafe wrapper might be nice here:

#include <ccan/typesafe_cb/typesafe_cb.h>

void coroutine_init_(struct coroutine_state *cs,
		    void (*fn)(void *), void *arg,
		    struct coroutine_stack *stack);

#define coroutine_init(cs, fn, arg, stack) \
	coroutine_init_((cs),						\
		       typesafe_cb(void, void *, (fn), (arg)),		\
		       (void *)(arg), (stack))

But generally, very nice.

Cheers,
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan