From: Rusty Russell Date: Thu, 28 Feb 2019 05:25:29 +0000 (+1030) Subject: ccan/io: keep always pointers to plans, not a linked list. X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=a5881c0deb210f56c2095366ae6cdabd5230d68d ccan/io: keep always pointers to plans, not a linked list. Duplex io_conns can be in the always list twice, and it was a source of bugs, but I didn't want to add a second list_node. Since there are not many always at once, it's better (and more space-efficient) to use a simple pointer array. Signed-off-by: Rusty Russell --- diff --git a/ccan/io/backend.h b/ccan/io/backend.h index c071c5d6..d877e44e 100644 --- a/ccan/io/backend.h +++ b/ccan/io/backend.h @@ -60,9 +60,6 @@ struct io_plan { struct io_conn { struct fd fd; - /* always list. */ - struct list_node always; - void (*finish)(struct io_conn *, void *arg); void *finish_arg; @@ -76,15 +73,14 @@ bool add_conn(struct io_conn *c); bool add_duplex(struct io_conn *c); void del_listener(struct io_listener *l); void cleanup_conn_without_close(struct io_conn *c); -void backend_new_always(struct io_conn *conn); +bool backend_new_always(struct io_plan *plan); void backend_new_plan(struct io_conn *conn); -void remove_from_always(struct io_conn *conn); void backend_plan_done(struct io_conn *conn); void backend_wake(const void *wait); void io_ready(struct io_conn *conn, int pollflags); -void io_do_always(struct io_conn *conn); +void io_do_always(struct io_plan *conn); void io_do_wakeup(struct io_conn *conn, enum io_direction dir); void *do_io_loop(struct io_conn **ready); #endif /* CCAN_IO_BACKEND_H */ diff --git a/ccan/io/io.c b/ccan/io/io.c index c5c9bee0..7ebd5a20 100644 --- a/ccan/io/io.c +++ b/ccan/io/io.c @@ -97,7 +97,6 @@ struct io_conn *io_new_conn_(const tal_t *ctx, int fd, conn->fd.fd = fd; conn->finish = NULL; conn->finish_arg = NULL; - list_node_init(&conn->always); if (!add_conn(conn)) return tal_free(conn); @@ -145,7 +144,9 @@ static struct io_plan *set_always(struct io_conn *conn, struct io_plan *plan = &conn->plan[dir]; plan->status = IO_ALWAYS; - backend_new_always(conn); + /* Only happens on OOM, and only with non-default tal_backend. */ + if (!backend_new_always(plan)) + return NULL; return io_set_plan(conn, dir, NULL, next, arg); } @@ -414,27 +415,14 @@ void io_ready(struct io_conn *conn, int pollflags) || conn->plan[IO_IN].status == IO_POLLING_STARTED); } -void io_do_always(struct io_conn *conn) +void io_do_always(struct io_plan *plan) { - /* There's a corner case where the in next_plan wakes up the - * out, placing it in IO_ALWAYS and we end up processing it immediately, - * only to leave it in the always list. - * - * Yet we can't just process one, in case they are both supposed - * to be done, so grab state beforehand. - */ - bool always_out = (conn->plan[IO_OUT].status == IO_ALWAYS); + struct io_conn *conn; - if (conn->plan[IO_IN].status == IO_ALWAYS) - if (!next_plan(conn, &conn->plan[IO_IN])) - return; + assert(plan->status == IO_ALWAYS); + conn = container_of(plan, struct io_conn, plan[plan->dir]); - if (always_out) { - /* You can't *unalways* a conn (except by freeing, in which - * case next_plan() returned false */ - assert(conn->plan[IO_OUT].status == IO_ALWAYS); - next_plan(conn, &conn->plan[IO_OUT]); - } + next_plan(conn, plan); } void io_do_wakeup(struct io_conn *conn, enum io_direction dir) diff --git a/ccan/io/poll.c b/ccan/io/poll.c index b005a97e..a853c870 100644 --- a/ccan/io/poll.c +++ b/ccan/io/poll.c @@ -11,11 +11,10 @@ #include #include -static size_t num_fds = 0, max_fds = 0, num_waiting = 0; +static size_t num_fds = 0, max_fds = 0, num_waiting = 0, num_always = 0, max_always = 0; static struct pollfd *pollfds = NULL; static struct fd **fds = NULL; -static LIST_HEAD(closing); -static LIST_HEAD(always); +static struct io_plan **always = NULL; static struct timemono (*nowfn)(void) = time_mono; static int (*pollfn)(struct pollfd *fds, nfds_t nfds, int timeout) = poll; @@ -110,16 +109,52 @@ bool add_listener(struct io_listener *l) return true; } -void remove_from_always(struct io_conn *conn) +static int find_always(const struct io_plan *plan) { - list_del_init(&conn->always); + for (size_t i = 0; i < num_always; i++) + if (always[i] == plan) + return i; + return -1; } -void backend_new_always(struct io_conn *conn) +static void remove_from_always(const struct io_plan *plan) { - /* In case it's already in always list. */ - list_del(&conn->always); - list_add_tail(&always, &conn->always); + int pos; + + if (plan->status != IO_ALWAYS) + return; + + pos = find_always(plan); + assert(pos >= 0); + + /* Move last one down if we made a hole */ + if (pos != num_always-1) + always[pos] = always[num_always-1]; + num_always--; +} + +bool backend_new_always(struct io_plan *plan) +{ + assert(find_always(plan) == -1); + + if (!max_always) { + assert(num_always == 0); + always = tal_arr(NULL, struct io_plan *, 8); + if (!always) + return false; + max_always = 8; + } + + if (num_always + 1 > max_always) { + size_t num = max_always * 2; + + if (!tal_resize(&always, num)) + return false; + max_always = num; + } + + always[num_always++] = plan; + return true; } void backend_new_plan(struct io_conn *conn) @@ -174,8 +209,9 @@ static void destroy_conn(struct io_conn *conn, bool close_fd) if (close_fd) close(conn->fd.fd); del_fd(&conn->fd); - /* In case it's on always list, remove it. */ - list_del_init(&conn->always); + + remove_from_always(&conn->plan[IO_IN]); + remove_from_always(&conn->plan[IO_OUT]); /* errno saved/restored by tal_free itself. */ if (conn->finish) { @@ -217,15 +253,12 @@ static void accept_conn(struct io_listener *l) static bool handle_always(void) { bool ret = false; - struct io_conn *conn; - - while ((conn = list_pop(&always, struct io_conn, always)) != NULL) { - assert(conn->plan[IO_IN].status == IO_ALWAYS - || conn->plan[IO_OUT].status == IO_ALWAYS); - /* Re-initialize, for next time. */ - list_node_init(&conn->always); - io_do_always(conn); + while (num_always > 0) { + /* Remove first: it might re-add */ + struct io_plan *plan = always[num_always-1]; + num_always--; + io_do_always(plan); ret = true; } return ret;