This diff makes proc.c daemons to use less file descriptors on startup,
this way we increase the number of child we can have considerably. This
also improves the solution on a bug reported in bugs@
"httpd errors out with 'too many open files'".
To achieve that I delayed the socket distribution and made a minimal
socket allocation in proc_init(), so only the necessary children socket
are allocated and passed with proc_exec(). After the event_init() is called
we call proc_connect() which creates the socketpair() and immediatly after
each call we already sends them without accumulating.
Note: We still have to calculate how many fds we will want to have and
then limit the daemon prefork configuration.
ok?
Index: proc.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.27
diff -u -p -r1.27 proc.c
--- proc.c 28 Sep 2016 12:01:04 -0000 1.27
+++ proc.c 4 Oct 2016 17:26:41 -0000
@@ -36,8 +36,6 @@
void proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
int, char **);
-void proc_connectpeer(struct privsep *, enum privsep_procid, int,
- struct privsep_pipes *);
void proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
void proc_open(struct privsep *, int, int);
void proc_accept(struct privsep *, int, enum privsep_procid,
@@ -147,72 +145,14 @@ proc_exec(struct privsep *ps, struct pri
}
void
-proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
- struct privsep_pipes *pp)
-{
- unsigned int i, j;
- struct privsep_fd pf;
-
- for (i = 0; i < PROC_MAX; i++) {
- /* Parent is already connected with everyone. */
- if (i == PROC_PARENT)
- continue;
-
- for (j = 0; j < ps->ps_instances[i]; j++) {
- /* Don't send socket to child itself. */
- if (i == (unsigned int)id &&
- j == (unsigned int)inst)
- continue;
- if (pp->pp_pipes[i][j] == -1)
- continue;
-
- pf.pf_procid = i;
- pf.pf_instance = j;
- proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
- -1, pp->pp_pipes[i][j], &pf, sizeof(pf));
- pp->pp_pipes[i][j] = -1;
- }
- }
-}
-
-/* Inter-connect all process except with ourself. */
-void
proc_connect(struct privsep *ps)
{
- unsigned int src, i, j;
- struct privsep_pipes *pp;
- struct imsgev *iev;
-
- /* Listen on appropriate pipes. */
- src = privsep_process;
- pp = &ps->ps_pipes[src][ps->ps_instance];
-
- for (i = 0; i < PROC_MAX; i++) {
- /* Don't listen to ourself. */
- if (i == src)
- continue;
-
- for (j = 0; j < ps->ps_instances[i]; j++) {
- if (pp->pp_pipes[i][j] == -1)
- continue;
-
- iev = &ps->ps_ievs[i][j];
- imsg_init(&iev->ibuf, pp->pp_pipes[i][j]);
- event_set(&iev->ev, iev->ibuf.fd, iev->events,
- iev->handler, iev->data);
- event_add(&iev->ev, NULL);
- }
- }
-
- /* Exchange pipes between process. */
- for (i = 0; i < PROC_MAX; i++) {
- /* Parent is already connected with everyone. */
- if (i == PROC_PARENT)
- continue;
+ unsigned int src, dst;
- for (j = 0; j < ps->ps_instances[i]; j++)
- proc_connectpeer(ps, i, j, &ps->ps_pipes[i][j]);
- }
+ /* Distribute the socketpair()s for everyone. */
+ for (src = 0; src < PROC_MAX; src++)
+ for (dst = src; dst < PROC_MAX; dst++)
+ proc_open(ps, src, dst);
}
void
@@ -220,17 +160,37 @@ proc_init(struct privsep *ps, struct pri
int argc, char **argv, enum privsep_procid proc_id)
{
struct privsep_proc *p = NULL;
+ struct privsep_pipes *pa, *pb;
unsigned int proc;
- unsigned int src, dst;
+ unsigned int dst;
+ int fds[2];
if (proc_id == PROC_PARENT) {
privsep_process = PROC_PARENT;
proc_setup(ps, procs, nproc);
- /* Open socketpair()s for everyone. */
- for (src = 0; src < PROC_MAX; src++)
- for (dst = 0; dst < PROC_MAX; dst++)
- proc_open(ps, src, dst);
+ /*
+ * Create the children sockets so we can use them
+ * to distribute the rest of the socketpair()s using
+ * proc_connect() later.
+ */
+ for (dst = 0; dst < PROC_MAX; dst++) {
+ /* Don't create socket for ourselves. */
+ if (dst == PROC_PARENT)
+ continue;
+
+ for (proc = 0; proc < ps->ps_instances[dst]; proc++) {
+ pa = &ps->ps_pipes[proc_id][ps->ps_instance];
+ pb = &ps->ps_pipes[dst][proc];
+ if (socketpair(AF_UNIX,
+ SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
+ PF_UNSPEC, fds) == -1)
+ fatal("socketpair");
+
+ pa->pp_pipes[dst][proc] = fds[0];
+ pb->pp_pipes[proc_id][ps->ps_instance] = fds[1];
+ }
+ }
/* Engage! */
proc_exec(ps, procs, nproc, argc, argv);
@@ -399,9 +359,16 @@ void
proc_open(struct privsep *ps, int src, int dst)
{
struct privsep_pipes *pa, *pb;
+ struct imsgev *iev;
+ struct privsep_fd pf;
int fds[2];
unsigned int i, j;
+ /* We don't have connections to the target process. */
+ if (ps->ps_ievs[dst] == NULL)
+ return;
+
+ /* Exchange pipes between process. */
for (i = 0; i < ps->ps_instances[src]; i++) {
for (j = 0; j < ps->ps_instances[dst]; j++) {
/* Don't create sockets for ourself. */
@@ -410,16 +377,63 @@ proc_open(struct privsep *ps, int src, i
pa = &ps->ps_pipes[src][i];
pb = &ps->ps_pipes[dst][j];
- if (pb->pp_pipes[dst][j] != -1)
+ if (pa->pp_pipes[dst][j] == -1 &&
+ pb->pp_pipes[src][i] == -1) {
+ if (socketpair(AF_UNIX,
+ SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
+ PF_UNSPEC, fds) == -1)
+ fatal("socketpair");
+
+ pa->pp_pipes[dst][j] = fds[0];
+ pb->pp_pipes[src][i] = fds[1];
+ }
+
+ iev = &ps->ps_ievs[dst][j];
+ if (iev->ibuf.fd == 0) {
+ imsg_init(&iev->ibuf, pa->pp_pipes[dst][j]);
+ event_set(&iev->ev, iev->ibuf.fd, iev->events,
+ iev->handler, iev->data);
+ event_add(&iev->ev, NULL);
+ }
+
+ /*
+ * The parent/child pipe was handed by proc_exec()
+ * and ps_ievs[PROC_PARENT] is not allocated here,
+ * so skip imsg_init() and fd send.
+ */
+ if (src == PROC_PARENT)
continue;
- if (socketpair(AF_UNIX,
- SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
- PF_UNSPEC, fds) == -1)
- fatal(__func__);
+ iev = &ps->ps_ievs[src][i];
+ if (iev->ibuf.fd == 0) {
+ imsg_init(&iev->ibuf, pa->pp_pipes[src][i]);
+ event_set(&iev->ev, iev->ibuf.fd, iev->events,
+ iev->handler, iev->data);
+ event_add(&iev->ev, NULL);
+ }
+
+ pf.pf_procid = src;
+ pf.pf_instance = i;
+ if (proc_compose_imsg(ps, dst, j, IMSG_CTL_PROCFD,
+ -1, pb->pp_pipes[src][i], &pf, sizeof(pf)) == -1)
+ fatal("proc_compose_imsg");
+
+ pf.pf_procid = dst;
+ pf.pf_instance = j;
+ if (proc_compose_imsg(ps, src, i, IMSG_CTL_PROCFD,
+ -1, pa->pp_pipes[dst][j], &pf, sizeof(pf)) == -1)
+ fatal("proc_compose_imsg");
+
+ /*
+ * We have to flush to send the descriptors and close
+ * them to avoid the fd ramp on startup.
+ */
+ if (imsg_flush(&ps->ps_ievs[dst][j].ibuf) == -1 ||
+ imsg_flush(&ps->ps_ievs[src][i].ibuf) == -1)
+ fatal("imsg_flush");
- pa->pp_pipes[dst][j] = fds[0];
- pb->pp_pipes[src][i] = fds[1];
+ pa->pp_pipes[dst][j] = -1;
+ pb->pp_pipes[src][i] = -1;
}
}
}
