[1/3] epoll: make sure all elements in ready list are in FIFO order
diff mbox series

Message ID 20181212110357.25656-2-rpenyaev@suse.de
State Superseded
Headers show
Series
  • use rwlock in order to reduce ep_poll_callback() contention
Related show

Commit Message

Roman Penyaev Dec. 12, 2018, 11:03 a.m. UTC
All coming events are stored in FIFO order and this is also should be
applicable to ->ovflist, which originally is stack, i.e. LIFO.

Thus to keep correct FIFO order ->ovflist should reversed by adding
elements to the head of the read list but not to the tail.

Signed-off-by: Roman Penyaev <rpenyaev@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 fs/eventpoll.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Davidlohr Bueso Dec. 13, 2018, 7:30 p.m. UTC | #1
On 2018-12-12 03:03, Roman Penyaev wrote:
> All coming events are stored in FIFO order and this is also should be
> applicable to ->ovflist, which originally is stack, i.e. LIFO.
> 
> Thus to keep correct FIFO order ->ovflist should reversed by adding
> elements to the head of the read list but not to the tail.

So the window for which the ovflist is used can be actually non-trivial 
(ie lots of copy_to_user) and I just hope nobody out there is relying on 
particular wakeup order. otoh nobody has every complained about this 
"reverse" order and not having the perfect queue. And hopefully the same 
will be for this case.

With that:

Reviewed-by: Davidlohr Bueso <dbueso@suse.de>

> Signed-off-by: Roman Penyaev <rpenyaev@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  fs/eventpoll.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index 2329f96469e2..3627c2e07149 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -722,7 +722,11 @@ static __poll_t ep_scan_ready_list(struct 
> eventpoll *ep,
>  		 * contain them, and the list_splice() below takes care of them.
>  		 */
>  		if (!ep_is_linked(epi)) {
> -			list_add_tail(&epi->rdllink, &ep->rdllist);
> +			/*
> +			 * ->ovflist is LIFO, so we have to reverse it in order
> +			 * to keep in FIFO.
> +			 */
> +			list_add(&epi->rdllink, &ep->rdllist);
>  			ep_pm_stay_awake(epi);
>  		}
>  	}

Patch
diff mbox series

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 2329f96469e2..3627c2e07149 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -722,7 +722,11 @@  static __poll_t ep_scan_ready_list(struct eventpoll *ep,
 		 * contain them, and the list_splice() below takes care of them.
 		 */
 		if (!ep_is_linked(epi)) {
-			list_add_tail(&epi->rdllink, &ep->rdllist);
+			/*
+			 * ->ovflist is LIFO, so we have to reverse it in order
+			 * to keep in FIFO.
+			 */
+			list_add(&epi->rdllink, &ep->rdllist);
 			ep_pm_stay_awake(epi);
 		}
 	}