On Mon, Feb 10 2014, Robert Baldyga wrote:
> This patch adds two example applications showing usage of Asynchronous I/O API
> of FunctionFS. First one (aio_simple) is simple example of bidirectional data
> transfer. Second one (aio_multibuff) shows multi-buffer data transfer, which
> may to be used in high performance applications.
>
> Both examples contains userspace applications for device and for host.
> It needs libaio library on the device, and libusb library on host.
>
> Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
> ---
>
> Hello,
>
> This is fourth version of patch adding examples of use AIO API of FunctionFS.
>
> In this version I have solved active polling problem replaceing it with select
> function. It's possible because AIO API supports eventfd notification when
> events are available to read. Using selecti() on ep0 file need wait_queue
> support in its poll funcion. It's done in v5 of my patchset adding AIO support
> to FunctionFS.
Thanks, this looks much better!
> I have also fixed error handling in host application, and add some
> modifications to get rid of duplicated code.
>
> tools/usb/aio_multibuff/device_app/aio_multibuff.c | 335 ++++++++++++++++++++
> tools/usb/aio_multibuff/host_app/Makefile | 13 +
> tools/usb/aio_multibuff/host_app/test.c | 146 +++++++++
> tools/usb/aio_simple/device_app/aio_simple.c | 325 +++++++++++++++++++
> tools/usb/aio_simple/host_app/Makefile | 13 +
> tools/usb/aio_simple/host_app/test.c | 148 +++++++++
Perhaps a single
tools/usb/ffs-aio-example
directory would be better? It's not really clear what “aio_simple”
means and that it relates to FFS.
> diff --git a/tools/usb/aio_multibuff/device_app/aio_multibuff.c b/tools/usb/aio_multibuff/device_app/aio_multibuff.c
> +int main(int argc, char *argv[])
> +{
> + int ret;
> + unsigned i, j;
> + char *ep_path;
> +
> + int ep0, ep1;
> +
> + io_context_t ctx;
> +
> + int evfd;
> + fd_set rfds;
> +
> + struct io_buffer iobuf[2];
> + int actual;
> + bool ready;
> +
> + if (argc != 2) {
> + printf("ffs directory not specified!\n");
> + return 1;
> + }
> +
> + ep_path = malloc(strlen(argv[1]) + 4 /* "/ep#" */ + 1 /* '\0' */);
> + if (!ep_path) {
> + perror("malloc");
> + return 1;
> + }
> +
> + /* open endpoint files */
> + sprintf(ep_path, "%s/ep0", argv[1]);
> + ep0 = open(ep_path, O_RDWR);
> + if (ep0 < 0) {
> + perror("unable to open ep0");
> + return 1;
> + }
> + if (write(ep0, &descriptors, sizeof(descriptors)) < 0) {
> + perror("unable do write descriptors");
> + return 1;
> + }
> + if (write(ep0, &strings, sizeof(strings)) < 0) {
> + perror("unable to write strings");
> + return 1;
> + }
> + sprintf(ep_path, "%s/ep1", argv[1]);
> + ep1 = open(ep_path, O_RDWR);
> + if (ep1 < 0) {
> + perror("unable to open ep1");
> + return 1;
> + }
> +
> + free(ep_path);
> +
> + memset(&ctx, 0, sizeof(ctx));
> + /* setup aio context to handle up to AIO_MAX requests */
> + if (io_setup(AIO_MAX, &ctx) < 0) {
> + perror("unable to setup aio");
> + return 1;
> + }
> +
> + evfd = eventfd(0, 0);
> + if (evfd < 0) {
> + perror("unable to open eventfd");
> + return 1;
> + }
> +
> + for (i = 0; i < sizeof(iobuf)/sizeof(*iobuf); ++i)
> + init_bufs(&iobuf[i], BUFS_MAX, BUF_LEN);
> +
> + while (1) {
> + FD_ZERO(&rfds);
> + FD_SET(ep0, &rfds);
> + FD_SET(evfd, &rfds);
> +
> + ret = select(((ep0 > evfd) ? ep0 : evfd)+1,
> + &rfds, NULL, NULL, NULL);
> + if (ret < 0)
> + continue;
Perhaps
+ if (ret < 0) {
+ if (errno == EINTR)
+ continue;
+ perror("select");
+ return 1;
+ }
or something similar.
> +
> + if (FD_ISSET(ep0, &rfds))
> + handle_ep0(ep0, &ready);
> +
> + /* we are waiting for function ENABLE */
> + if (!ready)
> + continue;
> +
> + /*
> + * when we're preparing new data to submit,
> + * second buffer being transmitted
> + */
> + for (i = 0; i < sizeof(iobuf)/sizeof(*iobuf); ++i) {
> + if (iobuf[i].requested)
> + continue;
> + /* if all req's from iocb1 completed */
“1” in the comment is not needed now, i.e. it should be:
+ /* if all reqs from iocb completed */
> + actual = (i+1)%(sizeof(iobuf)/sizeof(*iobuf));
I'm confused. Why is actual set here? I think it should be set to 0 at
the beginning and changed when events are consumed. Is there anything
I'm missing?
> + /* prepare requests */
> + for (j = 0; j < iobuf[i].cnt; ++j) {
> + io_prep_pwrite(iobuf[i].iocb[j], ep1,
> + iobuf[i].buf[j],
> + iobuf[i].len, 0);
> + /* enable eventfd notification */
> + iobuf[i].iocb[j]->u.c.flags |= IOCB_FLAG_RESFD;
> + iobuf[i].iocb[j]->u.c.resfd = evfd;
> + }
> + /* submit table of requests */
> + ret = io_submit(ctx, iobuf[i].cnt, iobuf[i].iocb);
> + if (ret >= 0) {
> + iobuf[i].requested = ret;
> + printf("submit: %d requests buf: %d\n", ret, i);
> + } else
> + perror("unable to submit reqests");
I would put body of the for loop in a separate function, but that's up
to you.
> + }
> + /* if something was submitted we wait for event */
> +
> + if (FD_ISSET(evfd, &rfds)) {
Perhaps
+ if (!FD_ISSET(evfd, &rfds))
+ continue;
to save indention level later on:
> + struct io_event e;
> + /* we wait for one event */
> + ret = io_getevents(ctx, 1, 1, &e, NULL);
> + if (ret > 0) /* if we got event */
> + iobuf[actual].requested--;
Again, I'm a bit confused here. Don't you have to read() from evfd?
Otherwise the next select() will return immediately. I think the whole
thing should be something like:
+ struct io_event e;
+ struct timespec timeout = { 0, 0 };
+ char buffer[8];
+ /* consume eventfd notification */
+ if (read(evfd, buffer, 8) < 2) {
+ perror("eventfd: read");
+ return -1;
+ }
+ /* consume IO events */
+ for (;;) {
+ ret = io_getevents(ctx, 1, 1, &e, &timeout);
+ if (ret <= 0)
+ break;
+ if (!iobuf[actual].requested)
+ actual = (actual + 1) %
+ (sizeof(iobuf)/sizeof(*iobuf));
+ iobuf[actual].requested--;
+ }
> + }
> + }
> +
> + /* free resources */
> +
> + for (i = 0; i < sizeof(iobuf)/sizeof(*iobuf); ++i)
> + delete_bufs(&iobuf[i]);
> + io_destroy(ctx);
> +
> + close(ep1);
> + close(ep0);
> +
> + return 0;
> +}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature
