[PATCH] perf record: Fix continue profiling after draining the buffer

[PATCH] perf record: Fix continue profiling after draining the buffer

[Yang Jihong]

commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
where the setting and checking of 'done' which add done_fd to pollfd.
When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
function returns a non-zero value.
As a result, perf record does not stop profiling.

The following simple scenarios can trigger this condition:

sleep 10 &
perf record -p $!

After the sleep process exits, perf record should stop profiling and exit.
However, perf record keeps running.

If pollfd revents contains only POLLERR or POLLHUP,
perf record indicates that buffer is draining and need to stop profiling.
Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
so that evlist__filter_pollfd does not filter and check done eventfd.

Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c    | 8 ++++++++
 tools/perf/util/evlist.h    | 4 ++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index fd3911650612..51e593e896ea 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		status = -1;
 		goto out_delete_session;
 	}
-	err = evlist__add_pollfd(rec->evlist, done_fd);
+	err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
 	if (err < 0) {
 		pr_err("Failed to add wakeup eventfd to poll list\n");
 		status = err;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 05363a7247c4..fea4c1e8010d 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
 	return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
 }
 
+#ifdef HAVE_EVENTFD_SUPPORT
+int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
+{
+	return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
+				       fdarray_flag__nonfilterable);
+}
+#endif
+
 int evlist__poll(struct evlist *evlist, int timeout)
 {
 	return perf_evlist__poll(&evlist->core, timeout);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 1aae75895dea..6d4d62151bc8 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -142,6 +142,10 @@ struct evsel *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
 int evlist__add_pollfd(struct evlist *evlist, int fd);
 int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask);
 
+#ifdef HAVE_EVENTFD_SUPPORT
+int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
+#endif
+
 int evlist__poll(struct evlist *evlist, int timeout);
 
 struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
-- 
2.17.1

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Namhyung Kim]

Hello,

On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
> where the setting and checking of 'done' which add done_fd to pollfd.
> When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
> function returns a non-zero value.
> As a result, perf record does not stop profiling.
>
> The following simple scenarios can trigger this condition:
>
> sleep 10 &
> perf record -p $!
>
> After the sleep process exits, perf record should stop profiling and exit.
> However, perf record keeps running.
>
> If pollfd revents contains only POLLERR or POLLHUP,
> perf record indicates that buffer is draining and need to stop profiling.
> Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
> so that evlist__filter_pollfd does not filter and check done eventfd.
>
> Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> ---
>  tools/perf/builtin-record.c | 2 +-
>  tools/perf/util/evlist.c    | 8 ++++++++
>  tools/perf/util/evlist.h    | 4 ++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index fd3911650612..51e593e896ea 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>                 status = -1;
>                 goto out_delete_session;
>         }
> -       err = evlist__add_pollfd(rec->evlist, done_fd);
> +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
>         if (err < 0) {
>                 pr_err("Failed to add wakeup eventfd to poll list\n");
>                 status = err;
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 05363a7247c4..fea4c1e8010d 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
>         return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
>  }
>
> +#ifdef HAVE_EVENTFD_SUPPORT
> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
> +{
> +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> +                                      fdarray_flag__nonfilterable);
> +}
> +#endif

Does it build when HAVE_EVENTFD_SUPPORT is not defined?

Thanks,
Namhyung


> +
>  int evlist__poll(struct evlist *evlist, int timeout)
>  {
>         return perf_evlist__poll(&evlist->core, timeout);
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 1aae75895dea..6d4d62151bc8 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -142,6 +142,10 @@ struct evsel *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
>  int evlist__add_pollfd(struct evlist *evlist, int fd);
>  int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask);
>
> +#ifdef HAVE_EVENTFD_SUPPORT
> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
> +#endif
> +
>  int evlist__poll(struct evlist *evlist, int timeout);
>
>  struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
> --
> 2.17.1
>

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Jiri Olsa]

On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
> Hello,
> 
> On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
> >
> > commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
> > where the setting and checking of 'done' which add done_fd to pollfd.
> > When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
> > function returns a non-zero value.
> > As a result, perf record does not stop profiling.
> >
> > The following simple scenarios can trigger this condition:
> >
> > sleep 10 &
> > perf record -p $!
> >
> > After the sleep process exits, perf record should stop profiling and exit.
> > However, perf record keeps running.
> >
> > If pollfd revents contains only POLLERR or POLLHUP,
> > perf record indicates that buffer is draining and need to stop profiling.
> > Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
> > so that evlist__filter_pollfd does not filter and check done eventfd.
> >
> > Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
> > Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> > ---
> >  tools/perf/builtin-record.c | 2 +-
> >  tools/perf/util/evlist.c    | 8 ++++++++
> >  tools/perf/util/evlist.h    | 4 ++++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index fd3911650612..51e593e896ea 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >                 status = -1;
> >                 goto out_delete_session;
> >         }
> > -       err = evlist__add_pollfd(rec->evlist, done_fd);
> > +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
> >         if (err < 0) {
> >                 pr_err("Failed to add wakeup eventfd to poll list\n");
> >                 status = err;
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index 05363a7247c4..fea4c1e8010d 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
> >         return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
> >  }
> >
> > +#ifdef HAVE_EVENTFD_SUPPORT
> > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
> > +{
> > +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> > +                                      fdarray_flag__nonfilterable);
> > +}
> > +#endif
> 
> Does it build when HAVE_EVENTFD_SUPPORT is not defined?

yea, I was wondering the same.. but it's called only from
code within HAVE_EVENTFD_SUPPORT ifdef

jirka

> 
> Thanks,
> Namhyung
> 
> 
> > +
> >  int evlist__poll(struct evlist *evlist, int timeout)
> >  {
> >         return perf_evlist__poll(&evlist->core, timeout);
> > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > index 1aae75895dea..6d4d62151bc8 100644
> > --- a/tools/perf/util/evlist.h
> > +++ b/tools/perf/util/evlist.h
> > @@ -142,6 +142,10 @@ struct evsel *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
> >  int evlist__add_pollfd(struct evlist *evlist, int fd);
> >  int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask);
> >
> > +#ifdef HAVE_EVENTFD_SUPPORT
> > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
> > +#endif
> > +
> >  int evlist__poll(struct evlist *evlist, int timeout);
> >
> >  struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
> > --
> > 2.17.1
> >
> 

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Yang Jihong]

Hello,

On 2021/2/5 18:46, Jiri Olsa wrote:
> On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
>> Hello,
>>
>> On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>>
>>> commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
>>> where the setting and checking of 'done' which add done_fd to pollfd.
>>> When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
>>> function returns a non-zero value.
>>> As a result, perf record does not stop profiling.
>>>
>>> The following simple scenarios can trigger this condition:
>>>
>>> sleep 10 &
>>> perf record -p $!
>>>
>>> After the sleep process exits, perf record should stop profiling and exit.
>>> However, perf record keeps running.
>>>
>>> If pollfd revents contains only POLLERR or POLLHUP,
>>> perf record indicates that buffer is draining and need to stop profiling.
>>> Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
>>> so that evlist__filter_pollfd does not filter and check done eventfd.
>>>
>>> Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
>>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>>> ---
>>>   tools/perf/builtin-record.c | 2 +-
>>>   tools/perf/util/evlist.c    | 8 ++++++++
>>>   tools/perf/util/evlist.h    | 4 ++++
>>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index fd3911650612..51e593e896ea 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>                  status = -1;
>>>                  goto out_delete_session;
>>>          }
>>> -       err = evlist__add_pollfd(rec->evlist, done_fd);
>>> +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
>>>          if (err < 0) {
>>>                  pr_err("Failed to add wakeup eventfd to poll list\n");
>>>                  status = err;
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index 05363a7247c4..fea4c1e8010d 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
>>>          return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
>>>   }
>>>
>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
>>> +{
>>> +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
>>> +                                      fdarray_flag__nonfilterable);
>>> +}
>>> +#endif
>>
>> Does it build when HAVE_EVENTFD_SUPPORT is not defined?
> 
> yea, I was wondering the same.. but it's called only from
> code within HAVE_EVENTFD_SUPPORT ifdef
> 
> jirka
> 
>>
>> Thanks,
>> Namhyung
>>
>>
I have tested that if "eventfd" feature is removed from the 
FEATURE_TESTS_BASIC list in Makefile.feature,
evlist__add_wakeup_eventfd will not be build.

Modify as follows:

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 97cbfb31b762..d1603050764a 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -32,7 +32,6 @@ FEATURE_TESTS_BASIC :=                  \
          backtrace                       \
          dwarf                           \
          dwarf_getlocations              \
-        eventfd                         \
          fortify-source                  \
          sync-compare-and-swap           \
          get_current_dir_name            \

Thanks,
Yang

>>> +
>>>   int evlist__poll(struct evlist *evlist, int timeout)
>>>   {
>>>          return perf_evlist__poll(&evlist->core, timeout);
>>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>>> index 1aae75895dea..6d4d62151bc8 100644
>>> --- a/tools/perf/util/evlist.h
>>> +++ b/tools/perf/util/evlist.h
>>> @@ -142,6 +142,10 @@ struct evsel *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
>>>   int evlist__add_pollfd(struct evlist *evlist, int fd);
>>>   int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask);
>>>
>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
>>> +#endif
>>> +
>>>   int evlist__poll(struct evlist *evlist, int timeout);
>>>
>>>   struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
>>> --
>>> 2.17.1
>>>
>>
> 
> .
> 

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Yang Jihong]

Hello,

On 2021/2/7 8:56, Yang Jihong wrote:
> Hello,
> 
> On 2021/2/5 18:46, Jiri Olsa wrote:
>> On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
>>> Hello,
>>>
>>> On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> 
>>> wrote:
>>>>
>>>> commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to 
>>>> solve rare race
>>>> where the setting and checking of 'done' which add done_fd to pollfd.
>>>> When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
>>>> function returns a non-zero value.
>>>> As a result, perf record does not stop profiling.
>>>>
>>>> The following simple scenarios can trigger this condition:
>>>>
>>>> sleep 10 &
>>>> perf record -p $!
>>>>
>>>> After the sleep process exits, perf record should stop profiling and 
>>>> exit.
>>>> However, perf record keeps running.
>>>>
>>>> If pollfd revents contains only POLLERR or POLLHUP,
>>>> perf record indicates that buffer is draining and need to stop 
>>>> profiling.
>>>> Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable 
>>>> objects,
>>>> so that evlist__filter_pollfd does not filter and check done eventfd.
>>>>
>>>> Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
>>>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>>>> ---
>>>>   tools/perf/builtin-record.c | 2 +-
>>>>   tools/perf/util/evlist.c    | 8 ++++++++
>>>>   tools/perf/util/evlist.h    | 4 ++++
>>>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>>> index fd3911650612..51e593e896ea 100644
>>>> --- a/tools/perf/builtin-record.c
>>>> +++ b/tools/perf/builtin-record.c
>>>> @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, 
>>>> int argc, const char **argv)
>>>>                  status = -1;
>>>>                  goto out_delete_session;
>>>>          }
>>>> -       err = evlist__add_pollfd(rec->evlist, done_fd);
>>>> +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
>>>>          if (err < 0) {
>>>>                  pr_err("Failed to add wakeup eventfd to poll list\n");
>>>>                  status = err;
>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>>> index 05363a7247c4..fea4c1e8010d 100644
>>>> --- a/tools/perf/util/evlist.c
>>>> +++ b/tools/perf/util/evlist.c
>>>> @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist 
>>>> *evlist, short revents_and_mask)
>>>>          return perf_evlist__filter_pollfd(&evlist->core, 
>>>> revents_and_mask);
>>>>   }
>>>>
>>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
>>>> +{
>>>> +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
>>>> +                                      fdarray_flag__nonfilterable);
>>>> +}
>>>> +#endif
>>>
>>> Does it build when HAVE_EVENTFD_SUPPORT is not defined?
>>
>> yea, I was wondering the same.. but it's called only from
>> code within HAVE_EVENTFD_SUPPORT ifdef
>>
>> jirka
>>
>>>
>>> Thanks,
>>> Namhyung
>>>
>>>
> I have tested that if "eventfd" feature is removed from the 
> FEATURE_TESTS_BASIC list in Makefile.feature,
> evlist__add_wakeup_eventfd will not be build.
> 
> Modify as follows:
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 97cbfb31b762..d1603050764a 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -32,7 +32,6 @@ FEATURE_TESTS_BASIC :=                  \
>           backtrace                       \
>           dwarf                           \
>           dwarf_getlocations              \
> -        eventfd                         \
>           fortify-source                  \
>           sync-compare-and-swap           \
>           get_current_dir_name            \
> 
> Thanks,
> Yang
> 
>>>> +
>>>>   int evlist__poll(struct evlist *evlist, int timeout)
>>>>   {
>>>>          return perf_evlist__poll(&evlist->core, timeout);
>>>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>>>> index 1aae75895dea..6d4d62151bc8 100644
>>>> --- a/tools/perf/util/evlist.h
>>>> +++ b/tools/perf/util/evlist.h
>>>> @@ -142,6 +142,10 @@ struct evsel 
>>>> *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
>>>>   int evlist__add_pollfd(struct evlist *evlist, int fd);
>>>>   int evlist__filter_pollfd(struct evlist *evlist, short 
>>>> revents_and_mask);
>>>>
>>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
>>>> +#endif
>>>> +
>>>>   int evlist__poll(struct evlist *evlist, int timeout);
>>>>
>>>>   struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
>>>> -- 
>>>> 2.17.1
>>>>
>>>
>>
>> .
>>
Do have any other questions about this patch?
Look forward to your review.

Thanks,
Yang

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Arnaldo Carvalho de Melo]

Em Fri, Feb 05, 2021 at 11:46:12AM +0100, Jiri Olsa escreveu:
> On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
> > Hello,
> > 
> > On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
> > >
> > > commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
> > > where the setting and checking of 'done' which add done_fd to pollfd.
> > > When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
> > > function returns a non-zero value.
> > > As a result, perf record does not stop profiling.
> > >
> > > The following simple scenarios can trigger this condition:
> > >
> > > sleep 10 &
> > > perf record -p $!
> > >
> > > After the sleep process exits, perf record should stop profiling and exit.
> > > However, perf record keeps running.
> > >
> > > If pollfd revents contains only POLLERR or POLLHUP,
> > > perf record indicates that buffer is draining and need to stop profiling.
> > > Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
> > > so that evlist__filter_pollfd does not filter and check done eventfd.
> > >
> > > Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
> > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> > > ---
> > >  tools/perf/builtin-record.c | 2 +-
> > >  tools/perf/util/evlist.c    | 8 ++++++++
> > >  tools/perf/util/evlist.h    | 4 ++++
> > >  3 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > index fd3911650612..51e593e896ea 100644
> > > --- a/tools/perf/builtin-record.c
> > > +++ b/tools/perf/builtin-record.c
> > > @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> > >                 status = -1;
> > >                 goto out_delete_session;
> > >         }
> > > -       err = evlist__add_pollfd(rec->evlist, done_fd);
> > > +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
> > >         if (err < 0) {
> > >                 pr_err("Failed to add wakeup eventfd to poll list\n");
> > >                 status = err;
> > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > > index 05363a7247c4..fea4c1e8010d 100644
> > > --- a/tools/perf/util/evlist.c
> > > +++ b/tools/perf/util/evlist.c
> > > @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
> > >         return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
> > >  }
> > >
> > > +#ifdef HAVE_EVENTFD_SUPPORT
> > > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
> > > +{
> > > +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> > > +                                      fdarray_flag__nonfilterable);
> > > +}
> > > +#endif
> > 
> > Does it build when HAVE_EVENTFD_SUPPORT is not defined?
> 
> yea, I was wondering the same.. but it's called only from
> code within HAVE_EVENTFD_SUPPORT ifdef

Yes, this can't work on systems without eventfd, it will simply not
build, and why do we have to make the definition of this function
conditional on HAVE_EVENTFD_SUPPORT?

I'm missing something :-\

Yeah, this whole call to evlist__add_pollfd is already surrounded by
#ifdef HAVE_EVENTFD_SUPPORT:

1656         if (zstd_init(&session->zstd_data, rec->opts.comp_level) < 0) {
1657                 pr_err("Compression initialization failed.\n");
1658                 return -1;
1659         }
1660 #ifdef HAVE_EVENTFD_SUPPORT
1661         done_fd = eventfd(0, EFD_NONBLOCK);
1662         if (done_fd < 0) {
1663                 pr_err("Failed to create wakeup eventfd, error: %m\n");
1664                 status = -1;
1665                 goto out_delete_session;
1666         }
1667         err = evlist__add_pollfd(rec->evlist, done_fd);
1668         if (err < 0) {
1669                 pr_err("Failed to add wakeup eventfd to poll list\n");
1670                 status = err;
1671                 goto out_delete_session;
1672         }
1673 #endif // HAVE_EVENTFD_SUPPORT
1674 
1675         session->header.env.comp_type  = PERF_COMP_ZSTD;
1676         session->header.env.comp_level = rec->opts.comp_level;

Jiri, does your Acked-by stands? Namhyung?

- Arnaldo
 
> jirka
> 
> > 
> > Thanks,
> > Namhyung
> > 
> > 
> > > +
> > >  int evlist__poll(struct evlist *evlist, int timeout)
> > >  {
> > >         return perf_evlist__poll(&evlist->core, timeout);
> > > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > > index 1aae75895dea..6d4d62151bc8 100644
> > > --- a/tools/perf/util/evlist.h
> > > +++ b/tools/perf/util/evlist.h
> > > @@ -142,6 +142,10 @@ struct evsel *evlist__find_tracepoint_by_name(struct evlist *evlist, const char
> > >  int evlist__add_pollfd(struct evlist *evlist, int fd);
> > >  int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask);
> > >
> > > +#ifdef HAVE_EVENTFD_SUPPORT
> > > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd);
> > > +#endif
> > > +
> > >  int evlist__poll(struct evlist *evlist, int timeout);
> > >
> > >  struct evsel *evlist__id2evsel(struct evlist *evlist, u64 id);
> > > --
> > > 2.17.1
> > >
> > 
> 

-- 

- Arnaldo

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Arnaldo Carvalho de Melo]

Em Thu, Feb 18, 2021 at 10:20:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Feb 05, 2021 at 11:46:12AM +0100, Jiri Olsa escreveu:
> > On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
> > > >
> > > > commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
> > > > where the setting and checking of 'done' which add done_fd to pollfd.
> > > > When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
> > > > function returns a non-zero value.
> > > > As a result, perf record does not stop profiling.
> > > >
> > > > The following simple scenarios can trigger this condition:
> > > >
> > > > sleep 10 &
> > > > perf record -p $!
> > > >
> > > > After the sleep process exits, perf record should stop profiling and exit.
> > > > However, perf record keeps running.
> > > >
> > > > If pollfd revents contains only POLLERR or POLLHUP,
> > > > perf record indicates that buffer is draining and need to stop profiling.
> > > > Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
> > > > so that evlist__filter_pollfd does not filter and check done eventfd.
> > > >
> > > > Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
> > > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> > > > ---
> > > >  tools/perf/builtin-record.c | 2 +-
> > > >  tools/perf/util/evlist.c    | 8 ++++++++
> > > >  tools/perf/util/evlist.h    | 4 ++++
> > > >  3 files changed, 13 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > > index fd3911650612..51e593e896ea 100644
> > > > --- a/tools/perf/builtin-record.c
> > > > +++ b/tools/perf/builtin-record.c
> > > > @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> > > >                 status = -1;
> > > >                 goto out_delete_session;
> > > >         }
> > > > -       err = evlist__add_pollfd(rec->evlist, done_fd);
> > > > +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
> > > >         if (err < 0) {
> > > >                 pr_err("Failed to add wakeup eventfd to poll list\n");
> > > >                 status = err;
> > > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > > > index 05363a7247c4..fea4c1e8010d 100644
> > > > --- a/tools/perf/util/evlist.c
> > > > +++ b/tools/perf/util/evlist.c
> > > > @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
> > > >         return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
> > > >  }
> > > >
> > > > +#ifdef HAVE_EVENTFD_SUPPORT
> > > > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
> > > > +{
> > > > +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> > > > +                                      fdarray_flag__nonfilterable);
> > > > +}
> > > > +#endif
> > > 
> > > Does it build when HAVE_EVENTFD_SUPPORT is not defined?
> > 
> > yea, I was wondering the same.. but it's called only from
> > code within HAVE_EVENTFD_SUPPORT ifdef
> 
> Yes, this can't work on systems without eventfd, it will simply not
> build, and why do we have to make the definition of this function
> conditional on HAVE_EVENTFD_SUPPORT?
> 
> I'm missing something :-\
> 
> Yeah, this whole call to evlist__add_pollfd is already surrounded by
> #ifdef HAVE_EVENTFD_SUPPORT:
> 
> 1656         if (zstd_init(&session->zstd_data, rec->opts.comp_level) < 0) {
> 1657                 pr_err("Compression initialization failed.\n");
> 1658                 return -1;
> 1659         }
> 1660 #ifdef HAVE_EVENTFD_SUPPORT
> 1661         done_fd = eventfd(0, EFD_NONBLOCK);
> 1662         if (done_fd < 0) {
> 1663                 pr_err("Failed to create wakeup eventfd, error: %m\n");
> 1664                 status = -1;
> 1665                 goto out_delete_session;
> 1666         }
> 1667         err = evlist__add_pollfd(rec->evlist, done_fd);
> 1668         if (err < 0) {
> 1669                 pr_err("Failed to add wakeup eventfd to poll list\n");
> 1670                 status = err;
> 1671                 goto out_delete_session;
> 1672         }
> 1673 #endif // HAVE_EVENTFD_SUPPORT
> 1674 
> 1675         session->header.env.comp_type  = PERF_COMP_ZSTD;
> 1676         session->header.env.comp_level = rec->opts.comp_level;
> 
> Jiri, does your Acked-by stands? Namhyung?

Thanks tested and applied, together with Jiri's Tested-by,

- Arnaldo

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Yang Jihong]

Hello,

On 2021/2/19 1:09, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 18, 2021 at 10:20:53AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Feb 05, 2021 at 11:46:12AM +0100, Jiri Olsa escreveu:
>>> On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
>>>> Hello,
>>>>
>>>> On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>>>>
>>>>> commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
>>>>> where the setting and checking of 'done' which add done_fd to pollfd.
>>>>> When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
>>>>> function returns a non-zero value.
>>>>> As a result, perf record does not stop profiling.
>>>>>
>>>>> The following simple scenarios can trigger this condition:
>>>>>
>>>>> sleep 10 &
>>>>> perf record -p $!
>>>>>
>>>>> After the sleep process exits, perf record should stop profiling and exit.
>>>>> However, perf record keeps running.
>>>>>
>>>>> If pollfd revents contains only POLLERR or POLLHUP,
>>>>> perf record indicates that buffer is draining and need to stop profiling.
>>>>> Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
>>>>> so that evlist__filter_pollfd does not filter and check done eventfd.
>>>>>
>>>>> Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
>>>>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>>>>> ---
>>>>>   tools/perf/builtin-record.c | 2 +-
>>>>>   tools/perf/util/evlist.c    | 8 ++++++++
>>>>>   tools/perf/util/evlist.h    | 4 ++++
>>>>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>>>> index fd3911650612..51e593e896ea 100644
>>>>> --- a/tools/perf/builtin-record.c
>>>>> +++ b/tools/perf/builtin-record.c
>>>>> @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>>>                  status = -1;
>>>>>                  goto out_delete_session;
>>>>>          }
>>>>> -       err = evlist__add_pollfd(rec->evlist, done_fd);
>>>>> +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
>>>>>          if (err < 0) {
>>>>>                  pr_err("Failed to add wakeup eventfd to poll list\n");
>>>>>                  status = err;
>>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>>>> index 05363a7247c4..fea4c1e8010d 100644
>>>>> --- a/tools/perf/util/evlist.c
>>>>> +++ b/tools/perf/util/evlist.c
>>>>> @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
>>>>>          return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
>>>>>   }
>>>>>
>>>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
>>>>> +{
>>>>> +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
>>>>> +                                      fdarray_flag__nonfilterable);
>>>>> +}
>>>>> +#endif
>>>>
>>>> Does it build when HAVE_EVENTFD_SUPPORT is not defined?
>>>
>>> yea, I was wondering the same.. but it's called only from
>>> code within HAVE_EVENTFD_SUPPORT ifdef
>>
>> Yes, this can't work on systems without eventfd, it will simply not
>> build, and why do we have to make the definition of this function
>> conditional on HAVE_EVENTFD_SUPPORT?
>>
>> I'm missing something :-\
>>
>> Yeah, this whole call to evlist__add_pollfd is already surrounded by
>> #ifdef HAVE_EVENTFD_SUPPORT:
>>
>> 1656         if (zstd_init(&session->zstd_data, rec->opts.comp_level) < 0) {
>> 1657                 pr_err("Compression initialization failed.\n");
>> 1658                 return -1;
>> 1659         }
>> 1660 #ifdef HAVE_EVENTFD_SUPPORT
>> 1661         done_fd = eventfd(0, EFD_NONBLOCK);
>> 1662         if (done_fd < 0) {
>> 1663                 pr_err("Failed to create wakeup eventfd, error: %m\n");
>> 1664                 status = -1;
>> 1665                 goto out_delete_session;
>> 1666         }
>> 1667         err = evlist__add_pollfd(rec->evlist, done_fd);
>> 1668         if (err < 0) {
>> 1669                 pr_err("Failed to add wakeup eventfd to poll list\n");
>> 1670                 status = err;
>> 1671                 goto out_delete_session;
>> 1672         }
>> 1673 #endif // HAVE_EVENTFD_SUPPORT
>> 1674
>> 1675         session->header.env.comp_type  = PERF_COMP_ZSTD;
>> 1676         session->header.env.comp_level = rec->opts.comp_level;
>>
>> Jiri, does your Acked-by stands? Namhyung?
> 
> Thanks tested and applied, together with Jiri's Tested-by,
> 
> - Arnaldo
> .
> 
Is this patch okay? Is there anything that needs to be modified?

Thanks,
Yang

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Arnaldo Carvalho de Melo]

Em Mon, Feb 22, 2021 at 09:31:51AM +0800, Yang Jihong escreveu:
> Hello,
> 
> On 2021/2/19 1:09, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Feb 18, 2021 at 10:20:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Fri, Feb 05, 2021 at 11:46:12AM +0100, Jiri Olsa escreveu:
> > > > On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
> > > > > Hello,
> > > > > 
> > > > > On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
> > > > > > 
> > > > > > commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
> > > > > > where the setting and checking of 'done' which add done_fd to pollfd.
> > > > > > When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
> > > > > > function returns a non-zero value.
> > > > > > As a result, perf record does not stop profiling.
> > > > > > 
> > > > > > The following simple scenarios can trigger this condition:
> > > > > > 
> > > > > > sleep 10 &
> > > > > > perf record -p $!
> > > > > > 
> > > > > > After the sleep process exits, perf record should stop profiling and exit.
> > > > > > However, perf record keeps running.
> > > > > > 
> > > > > > If pollfd revents contains only POLLERR or POLLHUP,
> > > > > > perf record indicates that buffer is draining and need to stop profiling.
> > > > > > Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
> > > > > > so that evlist__filter_pollfd does not filter and check done eventfd.
> > > > > > 
> > > > > > Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
> > > > > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> > > > > > ---
> > > > > >   tools/perf/builtin-record.c | 2 +-
> > > > > >   tools/perf/util/evlist.c    | 8 ++++++++
> > > > > >   tools/perf/util/evlist.h    | 4 ++++
> > > > > >   3 files changed, 13 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > > > > index fd3911650612..51e593e896ea 100644
> > > > > > --- a/tools/perf/builtin-record.c
> > > > > > +++ b/tools/perf/builtin-record.c
> > > > > > @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> > > > > >                  status = -1;
> > > > > >                  goto out_delete_session;
> > > > > >          }
> > > > > > -       err = evlist__add_pollfd(rec->evlist, done_fd);
> > > > > > +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
> > > > > >          if (err < 0) {
> > > > > >                  pr_err("Failed to add wakeup eventfd to poll list\n");
> > > > > >                  status = err;
> > > > > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > > > > > index 05363a7247c4..fea4c1e8010d 100644
> > > > > > --- a/tools/perf/util/evlist.c
> > > > > > +++ b/tools/perf/util/evlist.c
> > > > > > @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
> > > > > >          return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
> > > > > >   }
> > > > > > 
> > > > > > +#ifdef HAVE_EVENTFD_SUPPORT
> > > > > > +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
> > > > > > +{
> > > > > > +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> > > > > > +                                      fdarray_flag__nonfilterable);
> > > > > > +}
> > > > > > +#endif
> > > > > 
> > > > > Does it build when HAVE_EVENTFD_SUPPORT is not defined?
> > > > 
> > > > yea, I was wondering the same.. but it's called only from
> > > > code within HAVE_EVENTFD_SUPPORT ifdef
> > > 
> > > Yes, this can't work on systems without eventfd, it will simply not
> > > build, and why do we have to make the definition of this function
> > > conditional on HAVE_EVENTFD_SUPPORT?
> > > 
> > > I'm missing something :-\
> > > 
> > > Yeah, this whole call to evlist__add_pollfd is already surrounded by
> > > #ifdef HAVE_EVENTFD_SUPPORT:
> > > 
> > > 1656         if (zstd_init(&session->zstd_data, rec->opts.comp_level) < 0) {
> > > 1657                 pr_err("Compression initialization failed.\n");
> > > 1658                 return -1;
> > > 1659         }
> > > 1660 #ifdef HAVE_EVENTFD_SUPPORT
> > > 1661         done_fd = eventfd(0, EFD_NONBLOCK);
> > > 1662         if (done_fd < 0) {
> > > 1663                 pr_err("Failed to create wakeup eventfd, error: %m\n");
> > > 1664                 status = -1;
> > > 1665                 goto out_delete_session;
> > > 1666         }
> > > 1667         err = evlist__add_pollfd(rec->evlist, done_fd);
> > > 1668         if (err < 0) {
> > > 1669                 pr_err("Failed to add wakeup eventfd to poll list\n");
> > > 1670                 status = err;
> > > 1671                 goto out_delete_session;
> > > 1672         }
> > > 1673 #endif // HAVE_EVENTFD_SUPPORT
> > > 1674
> > > 1675         session->header.env.comp_type  = PERF_COMP_ZSTD;
> > > 1676         session->header.env.comp_level = rec->opts.comp_level;
> > > 
> > > Jiri, does your Acked-by stands? Namhyung?
> > 
> > Thanks tested and applied, together with Jiri's Tested-by,
> > 
> > - Arnaldo
> > .
> > 
> Is this patch okay? Is there anything that needs to be modified?

It was merged:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/tools/perf/builtin-record.c?id=e16c2ce7c5ed5de881066c1fd10ba5c09af69559

- Arnaldo

Re: [PATCH] perf record: Fix continue profiling after draining the buffer

[Yang Jihong]

On 2021/3/4 0:43, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 22, 2021 at 09:31:51AM +0800, Yang Jihong escreveu:
>> Hello,
>>
>> On 2021/2/19 1:09, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Feb 18, 2021 at 10:20:53AM -0300, Arnaldo Carvalho de Melo escreveu:
>>>> Em Fri, Feb 05, 2021 at 11:46:12AM +0100, Jiri Olsa escreveu:
>>>>> On Fri, Feb 05, 2021 at 07:35:22PM +0900, Namhyung Kim wrote:
>>>>>> Hello,
>>>>>>
>>>>>> On Fri, Feb 5, 2021 at 3:50 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>>>>>>
>>>>>>> commit da231338ec9c098707c8a1e4d8a50e2400e2fe17 uses eventfd to solve rare race
>>>>>>> where the setting and checking of 'done' which add done_fd to pollfd.
>>>>>>> When draining buffer, revents of done_fd is 0 and evlist__filter_pollfd
>>>>>>> function returns a non-zero value.
>>>>>>> As a result, perf record does not stop profiling.
>>>>>>>
>>>>>>> The following simple scenarios can trigger this condition:
>>>>>>>
>>>>>>> sleep 10 &
>>>>>>> perf record -p $!
>>>>>>>
>>>>>>> After the sleep process exits, perf record should stop profiling and exit.
>>>>>>> However, perf record keeps running.
>>>>>>>
>>>>>>> If pollfd revents contains only POLLERR or POLLHUP,
>>>>>>> perf record indicates that buffer is draining and need to stop profiling.
>>>>>>> Use fdarray_flag__nonfilterable to set done eventfd to nonfilterable objects,
>>>>>>> so that evlist__filter_pollfd does not filter and check done eventfd.
>>>>>>>
>>>>>>> Fixes: da231338ec9c (perf record: Use an eventfd to wakeup when done)
>>>>>>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>>>>>>> ---
>>>>>>>    tools/perf/builtin-record.c | 2 +-
>>>>>>>    tools/perf/util/evlist.c    | 8 ++++++++
>>>>>>>    tools/perf/util/evlist.h    | 4 ++++
>>>>>>>    3 files changed, 13 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>>>>>> index fd3911650612..51e593e896ea 100644
>>>>>>> --- a/tools/perf/builtin-record.c
>>>>>>> +++ b/tools/perf/builtin-record.c
>>>>>>> @@ -1663,7 +1663,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>>>>>                   status = -1;
>>>>>>>                   goto out_delete_session;
>>>>>>>           }
>>>>>>> -       err = evlist__add_pollfd(rec->evlist, done_fd);
>>>>>>> +       err = evlist__add_wakeup_eventfd(rec->evlist, done_fd);
>>>>>>>           if (err < 0) {
>>>>>>>                   pr_err("Failed to add wakeup eventfd to poll list\n");
>>>>>>>                   status = err;
>>>>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>>>>>> index 05363a7247c4..fea4c1e8010d 100644
>>>>>>> --- a/tools/perf/util/evlist.c
>>>>>>> +++ b/tools/perf/util/evlist.c
>>>>>>> @@ -572,6 +572,14 @@ int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
>>>>>>>           return perf_evlist__filter_pollfd(&evlist->core, revents_and_mask);
>>>>>>>    }
>>>>>>>
>>>>>>> +#ifdef HAVE_EVENTFD_SUPPORT
>>>>>>> +int evlist__add_wakeup_eventfd(struct evlist *evlist, int fd)
>>>>>>> +{
>>>>>>> +       return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
>>>>>>> +                                      fdarray_flag__nonfilterable);
>>>>>>> +}
>>>>>>> +#endif
>>>>>>
>>>>>> Does it build when HAVE_EVENTFD_SUPPORT is not defined?
>>>>>
>>>>> yea, I was wondering the same.. but it's called only from
>>>>> code within HAVE_EVENTFD_SUPPORT ifdef
>>>>
>>>> Yes, this can't work on systems without eventfd, it will simply not
>>>> build, and why do we have to make the definition of this function
>>>> conditional on HAVE_EVENTFD_SUPPORT?
>>>>
>>>> I'm missing something :-\
>>>>
>>>> Yeah, this whole call to evlist__add_pollfd is already surrounded by
>>>> #ifdef HAVE_EVENTFD_SUPPORT:
>>>>
>>>> 1656         if (zstd_init(&session->zstd_data, rec->opts.comp_level) < 0) {
>>>> 1657                 pr_err("Compression initialization failed.\n");
>>>> 1658                 return -1;
>>>> 1659         }
>>>> 1660 #ifdef HAVE_EVENTFD_SUPPORT
>>>> 1661         done_fd = eventfd(0, EFD_NONBLOCK);
>>>> 1662         if (done_fd < 0) {
>>>> 1663                 pr_err("Failed to create wakeup eventfd, error: %m\n");
>>>> 1664                 status = -1;
>>>> 1665                 goto out_delete_session;
>>>> 1666         }
>>>> 1667         err = evlist__add_pollfd(rec->evlist, done_fd);
>>>> 1668         if (err < 0) {
>>>> 1669                 pr_err("Failed to add wakeup eventfd to poll list\n");
>>>> 1670                 status = err;
>>>> 1671                 goto out_delete_session;
>>>> 1672         }
>>>> 1673 #endif // HAVE_EVENTFD_SUPPORT
>>>> 1674
>>>> 1675         session->header.env.comp_type  = PERF_COMP_ZSTD;
>>>> 1676         session->header.env.comp_level = rec->opts.comp_level;
>>>>
>>>> Jiri, does your Acked-by stands? Namhyung?
>>>
>>> Thanks tested and applied, together with Jiri's Tested-by,
>>>
>>> - Arnaldo
>>> .
>>>
>> Is this patch okay? Is there anything that needs to be modified?
> 
> It was merged:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/tools/perf/builtin-record.c?id=e16c2ce7c5ed5de881066c1fd10ba5c09af69559
> 
> - Arnaldo
> .
> Thanks.
Yang