linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
@ 2025-03-13  5:10 Tao Chen
  2025-03-13 10:05 ` Ingo Molnar
  0 siblings, 1 reply; 7+ messages in thread
From: Tao Chen @ 2025-03-13  5:10 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang
  Cc: linux-perf-users, linux-kernel, Tao Chen

The poll man page says POLLRDNORM is equivalent to POLLIN,
so add EPOLLRDNORM here.

Signed-off-by: Tao Chen <chen.dylane@linux.dev>
---
 kernel/events/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 59a52b1a1..5130b119d 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -19,7 +19,7 @@
 
 static void perf_output_wakeup(struct perf_output_handle *handle)
 {
-	atomic_set(&handle->rb->poll, EPOLLIN);
+	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);
 
 	handle->event->pending_wakeup = 1;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-13  5:10 [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll Tao Chen
@ 2025-03-13 10:05 ` Ingo Molnar
  2025-03-13 17:03   ` Tao Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2025-03-13 10:05 UTC (permalink / raw)
  To: Tao Chen
  Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel


* Tao Chen <chen.dylane@linux.dev> wrote:

> The poll man page says POLLRDNORM is equivalent to POLLIN,
> so add EPOLLRDNORM here.
> 
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> ---
>  kernel/events/ring_buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 59a52b1a1..5130b119d 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -19,7 +19,7 @@
>  
>  static void perf_output_wakeup(struct perf_output_handle *handle)
>  {
> -	atomic_set(&handle->rb->poll, EPOLLIN);
> +	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);

So what does EPOLLRDNORM mean to begin with? There doesn't seem to be 
separate/specific handling of it anywhere in the kernel that I can 
see...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-13 10:05 ` Ingo Molnar
@ 2025-03-13 17:03   ` Tao Chen
  2025-03-13 17:09     ` Ingo Molnar
  0 siblings, 1 reply; 7+ messages in thread
From: Tao Chen @ 2025-03-13 17:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel

在 2025/3/13 18:05, Ingo Molnar 写道:
> 
> * Tao Chen <chen.dylane@linux.dev> wrote:
> 
>> The poll man page says POLLRDNORM is equivalent to POLLIN,
>> so add EPOLLRDNORM here.
>>
>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>> ---
>>   kernel/events/ring_buffer.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
>> index 59a52b1a1..5130b119d 100644
>> --- a/kernel/events/ring_buffer.c
>> +++ b/kernel/events/ring_buffer.c
>> @@ -19,7 +19,7 @@
>>   
>>   static void perf_output_wakeup(struct perf_output_handle *handle)
>>   {
>> -	atomic_set(&handle->rb->poll, EPOLLIN);
>> +	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);
> 
> So what does EPOLLRDNORM mean to begin with? There doesn't seem to be
> separate/specific handling of it anywhere in the kernel that I can
> see...
> 

It seems that if user set pollfd with POLLRDNORM, perf_poll will not 
return until timeout even if perf_output_wakeup called, whereas POLLIN 
returns.

> Thanks,
> 
> 	Ingo


-- 
Best Regards
Tao Chen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-13 17:03   ` Tao Chen
@ 2025-03-13 17:09     ` Ingo Molnar
  2025-03-13 23:26       ` Tao Chen
  2025-03-14  2:31       ` Namhyung Kim
  0 siblings, 2 replies; 7+ messages in thread
From: Ingo Molnar @ 2025-03-13 17:09 UTC (permalink / raw)
  To: Tao Chen
  Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel


* Tao Chen <chen.dylane@linux.dev> wrote:

> 在 2025/3/13 18:05, Ingo Molnar 写道:
> > 
> > * Tao Chen <chen.dylane@linux.dev> wrote:
> > 
> > > The poll man page says POLLRDNORM is equivalent to POLLIN,
> > > so add EPOLLRDNORM here.
> > > 
> > > Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> > > ---
> > >   kernel/events/ring_buffer.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > > index 59a52b1a1..5130b119d 100644
> > > --- a/kernel/events/ring_buffer.c
> > > +++ b/kernel/events/ring_buffer.c
> > > @@ -19,7 +19,7 @@
> > >   static void perf_output_wakeup(struct perf_output_handle *handle)
> > >   {
> > > -	atomic_set(&handle->rb->poll, EPOLLIN);
> > > +	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);
> > 
> > So what does EPOLLRDNORM mean to begin with? There doesn't seem to be
> > separate/specific handling of it anywhere in the kernel that I can
> > see...
> > 
> 
> It seems that if user set pollfd with POLLRDNORM, perf_poll will not return
> until timeout even if perf_output_wakeup called, whereas POLLIN returns.

Mind adding this to the changelog, and explain that this patch fixes 
this particular poll() functionality and semantics for userspace?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-13 17:09     ` Ingo Molnar
@ 2025-03-13 23:26       ` Tao Chen
  2025-03-14  2:31       ` Namhyung Kim
  1 sibling, 0 replies; 7+ messages in thread
From: Tao Chen @ 2025-03-13 23:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel

在 2025/3/14 01:09, Ingo Molnar 写道:
> explain that this patch fixes
> this particular poll() functionality and semantics

will do it, thanks.

-- 
Best Regards
Tao Chen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-13 17:09     ` Ingo Molnar
  2025-03-13 23:26       ` Tao Chen
@ 2025-03-14  2:31       ` Namhyung Kim
  2025-03-14  8:39         ` Ingo Molnar
  1 sibling, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2025-03-14  2:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Tao Chen, peterz, mingo, acme, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel

Hello,

On Thu, Mar 13, 2025 at 06:09:45PM +0100, Ingo Molnar wrote:
> 
> * Tao Chen <chen.dylane@linux.dev> wrote:
> 
> > 在 2025/3/13 18:05, Ingo Molnar 写道:
> > > 
> > > * Tao Chen <chen.dylane@linux.dev> wrote:
> > > 
> > > > The poll man page says POLLRDNORM is equivalent to POLLIN,
> > > > so add EPOLLRDNORM here.
> > > > 
> > > > Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> > > > ---
> > > >   kernel/events/ring_buffer.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > > > index 59a52b1a1..5130b119d 100644
> > > > --- a/kernel/events/ring_buffer.c
> > > > +++ b/kernel/events/ring_buffer.c
> > > > @@ -19,7 +19,7 @@
> > > >   static void perf_output_wakeup(struct perf_output_handle *handle)
> > > >   {
> > > > -	atomic_set(&handle->rb->poll, EPOLLIN);
> > > > +	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);
> > > 
> > > So what does EPOLLRDNORM mean to begin with? There doesn't seem to be
> > > separate/specific handling of it anywhere in the kernel that I can
> > > see...
> > > 
> > 
> > It seems that if user set pollfd with POLLRDNORM, perf_poll will not return
> > until timeout even if perf_output_wakeup called, whereas POLLIN returns.
> 
> Mind adding this to the changelog, and explain that this patch fixes 
> this particular poll() functionality and semantics for userspace?

Off topic, but I think it should return something (either POLLHUP or
POLLERR) when the event goes to an error state like pinned events are
not scheduled.

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll
  2025-03-14  2:31       ` Namhyung Kim
@ 2025-03-14  8:39         ` Ingo Molnar
  0 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2025-03-14  8:39 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Tao Chen, peterz, mingo, acme, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, kan.liang, linux-perf-users,
	linux-kernel


* Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> On Thu, Mar 13, 2025 at 06:09:45PM +0100, Ingo Molnar wrote:
> > 
> > * Tao Chen <chen.dylane@linux.dev> wrote:
> > 
> > > 在 2025/3/13 18:05, Ingo Molnar 写道:
> > > > 
> > > > * Tao Chen <chen.dylane@linux.dev> wrote:
> > > > 
> > > > > The poll man page says POLLRDNORM is equivalent to POLLIN,
> > > > > so add EPOLLRDNORM here.
> > > > > 
> > > > > Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> > > > > ---
> > > > >   kernel/events/ring_buffer.c | 2 +-
> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > > > > index 59a52b1a1..5130b119d 100644
> > > > > --- a/kernel/events/ring_buffer.c
> > > > > +++ b/kernel/events/ring_buffer.c
> > > > > @@ -19,7 +19,7 @@
> > > > >   static void perf_output_wakeup(struct perf_output_handle *handle)
> > > > >   {
> > > > > -	atomic_set(&handle->rb->poll, EPOLLIN);
> > > > > +	atomic_set(&handle->rb->poll, EPOLLIN | EPOLLRDNORM);
> > > > 
> > > > So what does EPOLLRDNORM mean to begin with? There doesn't seem to be
> > > > separate/specific handling of it anywhere in the kernel that I can
> > > > see...
> > > > 
> > > 
> > > It seems that if user set pollfd with POLLRDNORM, perf_poll will not return
> > > until timeout even if perf_output_wakeup called, whereas POLLIN returns.
> > 
> > Mind adding this to the changelog, and explain that this patch fixes 
> > this particular poll() functionality and semantics for userspace?
> 
> Off topic, but I think it should return something (either POLLHUP or
> POLLERR) when the event goes to an error state like pinned events are
> not scheduled.

Mind sending a patch for that?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-03-14  8:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13  5:10 [PATCH linux-next] perf/ring_buffer: Add EPOLLRDNORM flag for poll Tao Chen
2025-03-13 10:05 ` Ingo Molnar
2025-03-13 17:03   ` Tao Chen
2025-03-13 17:09     ` Ingo Molnar
2025-03-13 23:26       ` Tao Chen
2025-03-14  2:31       ` Namhyung Kim
2025-03-14  8:39         ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).