Re: [PATCH 14/14] perf evlist: Unmap ring buffer when fd is nuked

[Jiri Olsa <jolsa@redhat.com>]

On Wed, Sep 10, 2014 at 11:08:49AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> +}
> +
>  int perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd)
>  {
> -	return fdarray__add(&evlist->pollfd, fd, POLLIN | POLLERR | POLLHUP);
> +	return __perf_evlist__add_pollfd(evlist, fd, -1);
> +}
> +
> +static void perf_evlist__munmap_filtered(struct fdarray *fda, int fd)
> +{
> +	struct perf_evlist *evlist = container_of(fda, struct perf_evlist, pollfd);
> +
> +	perf_evlist__mmap_put(evlist, fda->priv[fd]);

we cannot do this.. because of the way we read the map

getting error or hup does not mean the mmap is empty,
it can still have data, which we loose if we unmap it

following test will get data only with attached patch:

---
term1:
  $ cat

term2:
  $ cat perf record -p `pgrep cat`

term1:
  ^D
---

we get poll READ notification based on the wattermart settings,
which by default is half size of the ring buffer.. so for small
amount of perf data we dont get the poll read notification

I think we need to handle this in the record command context
and read out the mmap before we unmap it

jirka



>  }
>  
>  int perf_evlist__filter_pollfd(struct perf_evlist *evlist, short revents_and_mask)
>  {
> -	return fdarray__filter(&evlist->pollfd, revents_and_mask, NULL);
> +	return fdarray__filter(&evlist->pollfd, revents_and_mask,
> +			       perf_evlist__munmap_filtered);
>  }
>  
>  int perf_evlist__poll(struct perf_evlist *evlist, int timeout)
> @@ -751,7 +774,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
>  			perf_evlist__mmap_get(evlist, idx);
>  		}
>  
> -		if (perf_evlist__add_pollfd(evlist, fd) < 0) {
> +		if (__perf_evlist__add_pollfd(evlist, fd, idx) < 0) {
>  			perf_evlist__mmap_put(evlist, idx);
>  			return -1;
>  		}
> -- 
> 1.9.3
> 

---
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index fdb755f..9e71a47 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -448,6 +448,7 @@ static void perf_evlist__munmap_filtered(struct fdarray *fda, int fd)
 {
 	struct perf_evlist *evlist = container_of(fda, struct perf_evlist, pollfd);
 
+if (0)
 	perf_evlist__mmap_put(evlist, fda->priv[fd]);
 }