Re: perf: behavior of poll() changed in 3.18

[Jiri Olsa <jolsa@redhat.com>]

On Wed, Jan 21, 2015 at 01:06:30AM -0500, Vince Weaver wrote:
> On Tue, 20 Jan 2015, Jiri Olsa wrote:
> 
> > I made this change to get notification that monitored
> > process exited.  We use it in 'perf record' to find out
> > that we have nothing more to monitor and quit.
> > 
> > The logic is to return POLLHUP (via poll syscall) when
> > the monitored process exits for the event.
> > 
> > Could you please share your test code?
> 
> My code does something like this to count the number of overflows
> happening in a child:
> 
> 	/* fds contains one fd, which is a harware event */
> 	/* attached to a child */
> 
> 	while(1) {
>                 result=poll(fds,1,100);
>                 if (result==0) {
>                         waitpid(child,&status,WNOHANG);

		unrelated, but I noticed, that this seems broken.. you shouldn't check
		WIFEXITED(status) without waitpid(... WNOHANG) returning > 0

>                         if (WIFEXITED(status)) break;
>                         if (WIFSIGNALED(status)) {
>                                 printf("Signalled %d!\n",WTERMSIG(status));
>                                 break;
>                         }
>                 }
> 
>                 if (fds[0].revents&POLLIN) count.in++;
>                 if (fds[0].revents&POLLHUP) count.hup++;
>                 if (fds[0].revents&POLLERR) break;
> 	}
> 
> With the change you made this code hangs forever because poll no longer
> returns "0" when the child exits, but instead returns "1" with
> POLLHUP set.

right, once event is HUPed the poll will return POLLHUP in revents

> 
> I admit my code isn't the best out there, but it worked reliably up
> until 3.18 when the change was made.

I think that poll returning POLLHUP for an event which cease to work
due to its process died is correct.

On the other side we broke your code, which is against the policy.

However if we revert this code, we'll loose nice (and standard) way
to check if the event is still valid.. not sure how to handle this.

thoughts?

> 
> Part of why my code doesn't just exit on POLLHUP is because you can
> get that result for reasons other than a process exit (for example,
> if you are using ioctl(PERF_EVENT_IOC_REFRESH)

Nope, this is related to POOL_HUP (notice the '_') which you'll get
accompanied with SIGIO if you setup this.

Before you could get POLLHUP (through poll) only if you'd called
poll on unmapped event.

Now (after commit 179033b3e064) you'll get it also if the process
that the event monitored has died.


jirka