Date: Thu, 14 Apr 2016 17:14:11 +0000 (UTC) From: Eric van Gyzen <vangyzen@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r297977 - in stable/10: sys/kern sys/sys tests/sys/kqueue Message-ID: <201604141714.u3EHEBBR067170@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: vangyzen Date: Thu Apr 14 17:14:11 2016 New Revision: 297977 URL: https://svnweb.freebsd.org/changeset/base/297977 Log: MFC r295012 kqueue EVFILT_PROC: avoid collision between NOTE_CHILD and NOTE_EXIT NOTE_CHILD and NOTE_EXIT return something in kevent.data: the parent pid (ppid) for NOTE_CHILD and the exit status for NOTE_EXIT. Do not let the two events be combined, since one would overwrite the other's data. PR: 180385 Submitted by: David A. Bright <david_a_bright@dell.com> Sponsored by: Dell Inc. Modified: stable/10/sys/kern/kern_event.c stable/10/sys/sys/event.h stable/10/tests/sys/kqueue/common.h stable/10/tests/sys/kqueue/main.c stable/10/tests/sys/kqueue/proc.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/kern/kern_event.c ============================================================================== --- stable/10/sys/kern/kern_event.c Thu Apr 14 17:07:26 2016 (r297976) +++ stable/10/sys/kern/kern_event.c Thu Apr 14 17:14:11 2016 (r297977) @@ -364,11 +364,21 @@ filt_procattach(struct knote *kn) kn->kn_flags |= EV_CLEAR; /* automatically set */ /* - * internal flag indicating registration done by kernel + * Internal flag indicating registration done by kernel for the + * purposes of getting a NOTE_CHILD notification. */ - if (kn->kn_flags & EV_FLAG1) { + if (kn->kn_flags & EV_FLAG2) { + kn->kn_flags &= ~EV_FLAG2; kn->kn_data = kn->kn_sdata; /* ppid */ kn->kn_fflags = NOTE_CHILD; + kn->kn_sfflags &= ~NOTE_EXIT; + immediate = 1; /* Force immediate activation of child note. */ + } + /* + * Internal flag indicating registration done by kernel (for other than + * NOTE_CHILD). + */ + if (kn->kn_flags & EV_FLAG1) { kn->kn_flags &= ~EV_FLAG1; } @@ -376,9 +386,10 @@ filt_procattach(struct knote *kn) knlist_add(&p->p_klist, kn, 1); /* - * Immediately activate any exit notes if the target process is a - * zombie. This is necessary to handle the case where the target - * process, e.g. a child, dies before the kevent is registered. + * Immediately activate any child notes or, in the case of a zombie + * target process, exit notes. The latter is necessary to handle the + * case where the target process, e.g. a child, dies before the kevent + * is registered. */ if (immediate && filt_proc(kn, NOTE_EXIT)) KNOTE_ACTIVATE(kn, 0); @@ -487,7 +498,7 @@ knote_fork(struct knlist *list, int pid) /* * The NOTE_TRACK case. In addition to the activation - * of the event, we need to register new event to + * of the event, we need to register new events to * track the child. Drop the locks in preparation for * the call to kqueue_register(). */ @@ -496,8 +507,27 @@ knote_fork(struct knlist *list, int pid) list->kl_unlock(list->kl_lockarg); /* - * Activate existing knote and register a knote with + * Activate existing knote and register tracking knotes with * new process. + * + * First register a knote to get just the child notice. This + * must be a separate note from a potential NOTE_EXIT + * notification since both NOTE_CHILD and NOTE_EXIT are defined + * to use the data field (in conflicting ways). + */ + kev.ident = pid; + kev.filter = kn->kn_filter; + kev.flags = kn->kn_flags | EV_ADD | EV_ENABLE | EV_ONESHOT | EV_FLAG2; + kev.fflags = kn->kn_sfflags; + kev.data = kn->kn_id; /* parent */ + kev.udata = kn->kn_kevent.udata;/* preserve udata */ + error = kqueue_register(kq, &kev, NULL, 0); + if (error) + kn->kn_fflags |= NOTE_TRACKERR; + + /* + * Then register another knote to track other potential events + * from the new process. */ kev.ident = pid; kev.filter = kn->kn_filter; @@ -1071,7 +1101,7 @@ findkn: if (fp->f_type == DTYPE_KQUEUE) { /* - * if we add some inteligence about what we are doing, + * If we add some intelligence about what we are doing, * we should be able to support events on ourselves. * We need to know when we are doing this to prevent * getting both the knlist lock and the kq lock since @@ -1103,7 +1133,18 @@ findkn: kqueue_expand(kq, fops, kev->ident, waitok); KQ_LOCK(kq); - if (kq->kq_knhashmask != 0) { + + /* + * If possible, find an existing knote to use for this kevent. + */ + if (kev->filter == EVFILT_PROC && + (kev->flags & (EV_FLAG1 | EV_FLAG2)) != 0) { + /* This is an internal creation of a process tracking + * note. Don't attempt to coalesce this with an + * existing note. + */ + ; + } else if (kq->kq_knhashmask != 0) { struct klist *list; list = &kq->kq_knhash[ @@ -1115,7 +1156,7 @@ findkn: } } - /* knote is in the process of changing, wait for it to stablize. */ + /* knote is in the process of changing, wait for it to stabilize. */ if (kn != NULL && (kn->kn_status & KN_INFLUX) == KN_INFLUX) { KQ_GLOBAL_UNLOCK(&kq_global, haskqglobal); if (filedesc_unlock) { Modified: stable/10/sys/sys/event.h ============================================================================== --- stable/10/sys/sys/event.h Thu Apr 14 17:07:26 2016 (r297976) +++ stable/10/sys/sys/event.h Thu Apr 14 17:14:11 2016 (r297977) @@ -78,6 +78,7 @@ struct kevent { #define EV_SYSFLAGS 0xF000 /* reserved by system */ #define EV_DROP 0x1000 /* note should be dropped */ #define EV_FLAG1 0x2000 /* filter-specific flag */ +#define EV_FLAG2 0x4000 /* filter-specific flag */ /* returned values */ #define EV_EOF 0x8000 /* EOF detected */ Modified: stable/10/tests/sys/kqueue/common.h ============================================================================== --- stable/10/tests/sys/kqueue/common.h Thu Apr 14 17:07:26 2016 (r297976) +++ stable/10/tests/sys/kqueue/common.h Thu Apr 14 17:14:11 2016 (r297977) @@ -46,6 +46,7 @@ int vnode_fd; extern const char * kevent_to_str(struct kevent *); struct kevent * kevent_get(int); +struct kevent * kevent_get_timeout(int, int); void kevent_cmp(struct kevent *, struct kevent *); Modified: stable/10/tests/sys/kqueue/main.c ============================================================================== --- stable/10/tests/sys/kqueue/main.c Thu Apr 14 17:07:26 2016 (r297976) +++ stable/10/tests/sys/kqueue/main.c Thu Apr 14 17:14:11 2016 (r297977) @@ -69,6 +69,28 @@ kevent_get(int kqfd) return (kev); } +/* Retrieve a single kevent, specifying a maximum time to wait for it. */ +struct kevent * +kevent_get_timeout(int kqfd, int seconds) +{ + int nfds; + struct kevent *kev; + struct timespec timeout = {seconds, 0}; + + if ((kev = calloc(1, sizeof(*kev))) == NULL) + err(1, "out of memory"); + + nfds = kevent(kqfd, NULL, 0, kev, 1, &timeout); + if (nfds < 0) { + err(1, "kevent(2)"); + } else if (nfds == 0) { + free(kev); + kev = NULL; + } + + return (kev); +} + char * kevent_fflags_dump(struct kevent *kev) { @@ -82,25 +104,33 @@ kevent_fflags_dump(struct kevent *kev) abort(); /* Not every filter has meaningful fflags */ - if (kev->filter != EVFILT_VNODE) { - snprintf(buf, 1024, "fflags = %d", kev->fflags); - return (buf); - } - - snprintf(buf, 1024, "fflags = %d (", kev->fflags); - KEVFFL_DUMP(NOTE_DELETE); - KEVFFL_DUMP(NOTE_WRITE); - KEVFFL_DUMP(NOTE_EXTEND); + if (kev->filter == EVFILT_PROC) { + snprintf(buf, 1024, "fflags = %x (", kev->fflags); + KEVFFL_DUMP(NOTE_EXIT); + KEVFFL_DUMP(NOTE_FORK); + KEVFFL_DUMP(NOTE_EXEC); + KEVFFL_DUMP(NOTE_CHILD); + KEVFFL_DUMP(NOTE_TRACKERR); + KEVFFL_DUMP(NOTE_TRACK); + buf[strlen(buf) - 1] = ')'; + } else if (kev->filter == EVFILT_VNODE) { + snprintf(buf, 1024, "fflags = %x (", kev->fflags); + KEVFFL_DUMP(NOTE_DELETE); + KEVFFL_DUMP(NOTE_WRITE); + KEVFFL_DUMP(NOTE_EXTEND); #if HAVE_NOTE_TRUNCATE - KEVFFL_DUMP(NOTE_TRUNCATE); + KEVFFL_DUMP(NOTE_TRUNCATE); #endif - KEVFFL_DUMP(NOTE_ATTRIB); - KEVFFL_DUMP(NOTE_LINK); - KEVFFL_DUMP(NOTE_RENAME); + KEVFFL_DUMP(NOTE_ATTRIB); + KEVFFL_DUMP(NOTE_LINK); + KEVFFL_DUMP(NOTE_RENAME); #if HAVE_NOTE_REVOKE - KEVFFL_DUMP(NOTE_REVOKE); + KEVFFL_DUMP(NOTE_REVOKE); #endif - buf[strlen(buf) - 1] = ')'; + buf[strlen(buf) - 1] = ')'; + } else { + snprintf(buf, 1024, "fflags = %x", kev->fflags); + } return (buf); } @@ -260,6 +290,15 @@ main(int argc, char **argv) argc--; } + /* + * Some tests fork. If output is fully buffered, + * the children inherit some buffered data and flush + * it when they exit, causing some data to be printed twice. + * Use line buffering to avoid this problem. + */ + setlinebuf(stdout); + setlinebuf(stderr); + test_kqueue(); test_kqueue_close(); Modified: stable/10/tests/sys/kqueue/proc.c ============================================================================== --- stable/10/tests/sys/kqueue/proc.c Thu Apr 14 17:07:26 2016 (r297976) +++ stable/10/tests/sys/kqueue/proc.c Thu Apr 14 17:14:11 2016 (r297977) @@ -74,6 +74,172 @@ add_and_delete(void) } +static void +proc_track(int sleep_time) +{ + char test_id[64]; + struct kevent kev; + pid_t pid; + int pipe_fd[2]; + ssize_t result; + + snprintf(test_id, sizeof(test_id), + "kevent(EVFILT_PROC, NOTE_TRACK); sleep %d", sleep_time); + test_begin(test_id); + test_no_kevents(); + + if (pipe(pipe_fd)) { + err(1, "pipe (parent) failed! (%s() at %s:%d)", + __func__, __FILE__, __LINE__); + } + + /* Create a child to track. */ + pid = fork(); + if (pid == 0) { /* Child */ + pid_t grandchild = -1; + + /* + * Give the parent a chance to start tracking us. + */ + result = read(pipe_fd[1], test_id, 1); + if (result != 1) { + err(1, "read from pipe in child failed! (ret %zd) (%s() at %s:%d)", + result, __func__, __FILE__, __LINE__); + } + + /* + * Spawn a grandchild that will immediately exit. If the kernel has bug + * 180385, the parent will see a kevent with both NOTE_CHILD and + * NOTE_EXIT. If that bug is fixed, it will see two separate kevents + * for those notes. Note that this triggers the conditions for + * detecting the bug quite reliably on a 1 CPU system (or if the test + * process is restricted to a single CPU), but may not trigger it on a + * multi-CPU system. + */ + grandchild = fork(); + if (grandchild == 0) { /* Grandchild */ + if (sleep_time) sleep(sleep_time); + exit(1); + } else if (grandchild == -1) { /* Error */ + err(1, "fork (grandchild) failed! (%s() at %s:%d)", + __func__, __FILE__, __LINE__); + } else { /* Child (Grandchild Parent) */ + printf(" -- grandchild created (pid %d)\n", (int) grandchild); + } + if (sleep_time) sleep(sleep_time); + exit(0); + } else if (pid == -1) { /* Error */ + err(1, "fork (child) failed! (%s() at %s:%d)", + __func__, __FILE__, __LINE__); + } + + printf(" -- child created (pid %d)\n", (int) pid); + + kevent_add(kqfd, &kev, pid, EVFILT_PROC, EV_ADD | EV_ENABLE, + NOTE_TRACK | NOTE_EXEC | NOTE_EXIT | NOTE_FORK, + 0, NULL); + + printf(" -- tracking child (pid %d)\n", (int) pid); + + /* Now that we're tracking the child, tell it to proceed. */ + result = write(pipe_fd[0], test_id, 1); + if (result != 1) { + err(1, "write to pipe in parent failed! (ret %zd) (%s() at %s:%d)", + result, __func__, __FILE__, __LINE__); + } + + /* + * Several events should be received: + * - NOTE_FORK (from child) + * - NOTE_CHILD (from grandchild) + * - NOTE_EXIT (from grandchild) + * - NOTE_EXIT (from child) + * + * The NOTE_FORK and NOTE_EXIT from the child could be combined into a + * single event, but the NOTE_CHILD and NOTE_EXIT from the grandchild must + * not be combined. + * + * The loop continues until no events are received within a 5 second + * period, at which point it is assumed that no more will be coming. The + * loop is deliberately designed to attempt to get events even after all + * the expected ones are received in case some spurious events are + * generated as well as the expected ones. + */ + { + int child_exit = 0; + int child_fork = 0; + int gchild_exit = 0; + int gchild_note = 0; + pid_t gchild_pid = -1; + int done = 0; + + while (!done) + { + int handled = 0; + struct kevent *kevp; + + kevp = kevent_get_timeout(kqfd, 5); + if (kevp == NULL) { + done = 1; + } else { + printf(" -- Received kevent: %s\n", kevent_to_str(kevp)); + + if ((kevp->fflags & NOTE_CHILD) && (kevp->fflags & NOTE_EXIT)) { + errx(1, "NOTE_CHILD and NOTE_EXIT in same kevent: %s", kevent_to_str(kevp)); + } + + if (kevp->fflags & NOTE_CHILD) { + if (kevp->data == pid) { + if (!gchild_note) { + ++gchild_note; + gchild_pid = kevp->ident; + ++handled; + } else { + errx(1, "Spurious NOTE_CHILD: %s", kevent_to_str(kevp)); + } + } + } + + if (kevp->fflags & NOTE_EXIT) { + if ((kevp->ident == pid) && (!child_exit)) { + ++child_exit; + ++handled; + } else if ((kevp->ident == gchild_pid) && (!gchild_exit)) { + ++gchild_exit; + ++handled; + } else { + errx(1, "Spurious NOTE_EXIT: %s", kevent_to_str(kevp)); + } + } + + if (kevp->fflags & NOTE_FORK) { + if ((kevp->ident == pid) && (!child_fork)) { + ++child_fork; + ++handled; + } else { + errx(1, "Spurious NOTE_FORK: %s", kevent_to_str(kevp)); + } + } + + if (!handled) { + errx(1, "Spurious kevent: %s", kevent_to_str(kevp)); + } + + free(kevp); + } + } + + /* Make sure all expected events were received. */ + if (child_exit && child_fork && gchild_exit && gchild_note) { + printf(" -- Received all expected events.\n"); + } else { + errx(1, "Did not receive all expected events."); + } + } + + success(); +} + #ifdef TODO static void event_trigger(void) @@ -236,6 +402,8 @@ test_evfilt_proc() signal(SIGUSR1, sig_handler); add_and_delete(); + proc_track(0); /* Run without sleeping before children exit. */ + proc_track(1); /* Sleep a bit in the children before exiting. */ #if TODO event_trigger();
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201604141714.u3EHEBBR067170>