On Thu, Apr 28, 2022 at 09:38:10AM -0700, Sargun Dhillon wrote:
> On Thu, Apr 28, 2022 at 6:15 AM Tycho Andersen <tycho@tycho.pizza> wrote:
> > > + for (i = 0; i < ARRAY_SIZE(pids); i++) {
> > > + pid = fork();
> > > + if (pid == 0) {
> > > + ret = syscall(__NR_getppid);
> > > + exit(ret != USER_NOTIF_MAGIC);
> > > + }
> > > + pids[i] = pid;
> > > + }
> > > +
> > > + /* This spins until all of the children are sleeping */
> > > +restart_wait:
> > > + for (i = 0; i < ARRAY_SIZE(pids); i++) {
> > > + if (get_proc_stat(pids[i]) != 'S') {
> > > + nanosleep(&delay, NULL);
> > > + goto restart_wait;
> > > + }
> > > + }
> >
> > I wonder if we should/can combine this loop with the previous one, and
> > wait for the child to sleep in getppid() before we fork the next one.
> > Otherwise isn't racy in the case that your loop continues to the next
> > iteration before the child processes are scheduled, so things might be
> > out of order? Maybe I'm missing something.
> >
> > In any case, this change seems reasonable to me.
> >
> > Tycho
> It's okay if the child processes are started out of order. The test just
> verifies that the calls are delivered in FIFO order according to when
> the syscall was called (not when the process was started), and we do
> this by just looking at the notification ID. It doesn't care about which
> process generated the notification.
I totally missed that you don't this, I just assumed you did. Thanks.
Anyway, you can add:
Acked-by: Tycho Andersen <tycho@tycho.pizza>
to both patches.
Tycho
