Re: RFC: async I/O facility: "zpoll" API


Benjamin Walsh <benjamin.walsh@...>
 

On Mon, Sep 19, 2016 at 01:22:14PM -0700, Andy Ross wrote:
So I've been looking at the requirement for an async I/O framework in
ZEP-540, and this is the API I have so far. An implementation is
prototyped as far as working with a minimal test of a faked device,
I'll get that cleaned up and in gerrit for a review within a day. But
here's the API I'm looking at. Please tear it apart as appropriate;
it's an API we'll be stuck with for a while. Stuff to note:

It works a little like Unix select()/poll(), in that the caller gets
handles to a bunch of "zpoll_event" structures which are 1:1 with
async requests made (e.g. they're a return value from a putative
uart_write_async() call), and can wait on the full set of them, waking
up with any one of them has a completion event to signal.

Driver's are responsible for managing one of these events per
simultaneous outstanding request they want to handle (in mose cases
that number of requests will be one), and calling a straightforward
signal() function when the request is complete (or in an error state).

Because mixing blocking styles is madness (c.f. the historical
disaster of pthreads and file descriptors not being waitable in the
same call and the need for hacks like signalfd), it also accepts basic
semaphores as a waitable object. We might consider some of the other
IPC primitives for the same treatment.

The original request was for a callback mechanism, which I resisted.
But given this mechanism and the existance of the system work queue
(which, to be fair, is a heavyweight feature), that's not hard to
implement. So this supports both styles, but the callback one is
optional for apps with their own main loops that don't want to have a
extra 1kb thread stack sitting around just for event processing.

Semantically, this overlaps in a kind of ugly way with
sem_group_take(), which I see a patch in the queue for. Essentially
zpoll() is a proper superset of that facility. We should probably try
to unify/deprecate such that we can do things in only one way.

Please review. An implementation prototype will arrive promptly.
OK, this is _very_ interesting. We were thinking of actuatlly
implementing a 'wait on multiple objects' in the kernel rather than just
this 'semaphore group' we currently have (for backwards compatibility
for now anyway).

I'm really eager to see the prototype.

A couple of nitpicks:

- I don't think the documentation should focus on drivers. The is usable
by applications as well.

- This should be a native kernel feature. It should probably be called
k_poll rather than zpoll.

More comments below.


Andy


========================================================================

typedef void (*zpoll_callback_t)(int result, void *user_data);

enum zpoll_req_type { ZPOLL_REQ_POLL, ZPOLL_REQ_SEM, ZPOLL_REQ_CALLBACK };

struct zpoll_event {
enum zpoll_req_type type : 8;
int signaled : 1; /* Driver/source operation complete */
int active : 1; /* result not yet retrieved/delivered */
int result;
union {
struct {
zpoll_callback_t callback;
void *user_data;
Shouldn't we have a pointer to a workqueue, so that the work could be
scheduled on another workqueue than the system one if desired ?

} cb;
struct {
Shouldn't this be a union since one zpoll event instance should be for
one object ?

struct zpoll_call *poller;
Where is zpoll_call defined ? Isn't that simply a k_tid_t ?

struct k_sem *semaphore;
} polled;
};
};

/**
* @brief Re/initialize zpoll event
*
* For use by drivers who are initiating a new request. Will return
* an error (-EAGAIN) if the event is still in use, which should
* generally be either propagated back to the user code making the
* request or used to initiate some kind of queuing/allocation of a
* new event. Drivers should *not* pend or busy-wait at request time
* to "wait" for this to become available. These are asynchronous
* requests!
*
* @param evt A potentially-in-use zpoll_event structure
*/
int zpoll_event_init(struct zpoll_event *evt);

#ifdef ZPOLL_CALLBACKS
/**
* @brief Re/initialize zpoll event for a user-specified callback
*
* As for zpoll_event_init(), but assigns the callback fields where provided.
*
* @param evt A potentitally-in-use zpoll_event structure
* @param cb A user-provided callback function
* @param user_data A user-provided opaque pointer to be passed as the
* user_data argument to the callback
*/
int zpoll_event_init_cb(struct zpoll_event *evt, zpoll_callback_t cb, void *user_data);
#endif

/**
* @brief Static initializer for a semaphore zpoll_event
*
* Constructs a zpoll_event stucture suitable for passing to zpoll().
*
* @param sem A pointer to a valid k_sem_t.
*/
#define ZPOLL_EVENT_SEMAPHORE(sem) \
{ \
.request_type = ZPOLL_REQ_SEM, \
.signaled = 0, \
.semaphore = (sem), \
}

/**
* @brief Executes the user-provided handler for a complete event
*
* Called by driver code when the request abstracted by this zpoll
* event is complete, either by waking up a pending zpoll() call or
* scheduling a work thread to issue the user-provided callback.
*
* @param evt A zpoll_event structure whose action is complete
* @param result The integer result code of the asynchronous call
*/
void zpoll_signal(struct zpoll_event *evt, int result);

void _zpoll_sem_signal(struct k_sem *sem);
Do we need this ? Shouldn't this be the job of k_sem_give() to handle
waking up a thread from its polling ? I suppose that you do not have to
give a semaphore knowing that it is waiting on by a thread using zpoll,
right ?

Or is this a hook to be called from k_sem_give(). It might be, since it
is named like an internal kernel function.

Hard to tell without documentation.

Regards,
Ben


/**
* @brief Wait for the completion of any of a number of events
*
* Will pend until any of the zpoll_event structures receives a
* zpoll_signal() call from another context (or, for semaphores, a
* "give" event), or until a the specified timeout occurs.
*
* For convenience, will return one known-complete event pointer. But
* note that completion notifications are "level triggered" (they stay
* complete and will synchronously wake up future zpoll calls until
* their results are retrieved), and that there may be more whose
* state can be tested with zpoll_get_result(). In general most
* applications will simply loop on zpoll() getting one event at a
* time.
*
* @param events An array of zpoll_event structures
* @param nevents The size of the events array
* @param timeout_ticks How long to wait before returning, or
* TICKS_UNLIMITED to wait forever.
*
* @return One complete event, or NULL if the timeout expired.
*/
struct zpoll_event* zpoll(struct zpoll_event **events, int nevents,
int timeout_ticks);

/**
* @brief Retrieve the result code of a complete zpoll_event and mark
* it available
*
* Returns the integer result code of the operation encapsulated by
* the zpoll_event. For driver-managed events, this also marks the
* zpoll_event available for future asynchronous I/O calls and so
* *must* be called to free the resource for future I/O.
*
* Will return -EAGAIN if the event is not yet complete.
*
* @return The result code of the asynchronous operation
*/
int zpoll_get_result(struct zpoll_event *e);

Join devel@lists.zephyrproject.org to automatically receive all group messages.