Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[richard -rw- weinberger]

On Wed, Mar 28, 2012 at 3:57 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> High performance server sometimes create one listening socket (e.g. port
> 80), create a epoll file descriptor and add the socket. Afterwards
> create SC_NPROCESSORS_ONLN threads and wait for events. This often
> result in a thundering herd problem because all CPUs are scheduled.
>
> This patch add an additional flag to epoll_ctl(2) called EPOLLEXCLUSIVE.
> If a descriptor is added with this flag only one CPU is scheduled in.
>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
> ---
> Dave rejected the patch and said not network specific. Because there
> is no epoll maintainer this time directly.

CC'ing maintainers for you...
Please use scripts/get_maintainer.pl.

-- 
Thanks,
//richard

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Jason Baron]

On Wed, Mar 28, 2012 at 04:09:24PM +0200, richard -rw- weinberger wrote:
> On Wed, Mar 28, 2012 at 3:57 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> > High performance server sometimes create one listening socket (e.g. port
> > 80), create a epoll file descriptor and add the socket. Afterwards
> > create SC_NPROCESSORS_ONLN threads and wait for events. This often
> > result in a thundering herd problem because all CPUs are scheduled.
> >
> > This patch add an additional flag to epoll_ctl(2) called EPOLLEXCLUSIVE.
> > If a descriptor is added with this flag only one CPU is scheduled in.
> >
> > Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
> > ---
> > Dave rejected the patch and said not network specific. Because there
> > is no epoll maintainer this time directly.
> 
> CC'ing maintainers for you...
> Please use scripts/get_maintainer.pl.
> 

Hmmm...Looking at ep_poll() it does an '__add_wait_queue_exclusive()'.
So, I *think* epoll_wait() should do what you want, if you are waiting
on the same epfd in all the threads.

I think the case you are describing is where each thread does its own
ep_create(), and then a subsequent epoll_wait() on the fd from the
create?

So, I *think* you can get what you want without adding this flag.

Thanks,

-Jason

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Hagen Paul Pfeifer]

* Jason Baron | 2012-03-28 12:21:08 [-0400]:

>Hmmm...Looking at ep_poll() it does an '__add_wait_queue_exclusive()'.
>So, I *think* epoll_wait() should do what you want, if you are waiting
>on the same epfd in all the threads.
>
>I think the case you are describing is where each thread does its own
>ep_create(), and then a subsequent epoll_wait() on the fd from the
>create?
>
>So, I *think* you can get what you want without adding this flag.

;) sorry:

epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned
epoll_wait returned


minimal example:

>>>>>>>>>>>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/epoll.h>

#define	AMAX 16

static void *runner(void *args)
{
	int fd = (int) *((int *) args);
	struct epoll_event events[AMAX];

	epoll_wait(fd, events, AMAX, -1);
	write(1, "epoll_wait returned\n", 20);

	return NULL;
}

int main(int ac, char **av)
{
	int i, evfd, pipefd[2];
	pthread_t thread_id[2];
	struct epoll_event epoll_ev;

	pipe(pipefd);
	evfd = epoll_create(64);

	memset(&epoll_ev, 0, sizeof(struct epoll_event));
	epoll_ev.events   = EPOLLIN | EPOLLPRI | EPOLLERR | EPOLLHUP;
	epoll_ctl(evfd, EPOLL_CTL_ADD, pipefd[0], &epoll_ev);

	for (i = 0; i < 10; i++)
		pthread_create(&thread_id[0], NULL, runner, &evfd);

	sleep(1);
	close(pipefd[1]);
	write(pipefd[0], "x", 1);
	sleep(1);

	return EXIT_SUCCESS;
}
<<<<<<<<<<<


Cheers, Hagen

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Jason Baron]

On Wed, Mar 28, 2012 at 09:58:48PM +0200, Hagen Paul Pfeifer wrote:
> 
> >Hmmm...Looking at ep_poll() it does an '__add_wait_queue_exclusive()'.
> >So, I *think* epoll_wait() should do what you want, if you are waiting
> >on the same epfd in all the threads.
> >
> >I think the case you are describing is where each thread does its own
> >ep_create(), and then a subsequent epoll_wait() on the fd from the
> >create?
> >
> >So, I *think* you can get what you want without adding this flag.
> 
> ;) sorry:
> 
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> epoll_wait returned
> 
> 
> minimal example:
> 
> >>>>>>>>>>>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <pthread.h>
> #include <sys/epoll.h>
> 
> #define	AMAX 16
> 
> static void *runner(void *args)
> {
> 	int fd = (int) *((int *) args);
> 	struct epoll_event events[AMAX];
> 
> 	epoll_wait(fd, events, AMAX, -1);
> 	write(1, "epoll_wait returned\n", 20);
> 
> 	return NULL;
> }
> 
> int main(int ac, char **av)
> {
> 	int i, evfd, pipefd[2];
> 	pthread_t thread_id[2];
> 	struct epoll_event epoll_ev;
> 
> 	pipe(pipefd);
> 	evfd = epoll_create(64);
> 
> 	memset(&epoll_ev, 0, sizeof(struct epoll_event));
> 	epoll_ev.events   = EPOLLIN | EPOLLPRI | EPOLLERR | EPOLLHUP;
> 	epoll_ctl(evfd, EPOLL_CTL_ADD, pipefd[0], &epoll_ev);
> 
> 	for (i = 0; i < 10; i++)
> 		pthread_create(&thread_id[0], NULL, runner, &evfd);
> 
> 	sleep(1);
> 	close(pipefd[1]);
> 	write(pipefd[0], "x", 1);
> 	sleep(1);
> 
> 	return EXIT_SUCCESS;
> }

Right, for level triggered events, they all wait up. However, if you use
edge triggered, ie add 'EPOLLET', then the event gets 'consumed' by the
first thread that wakes up, and the subseqent waiters wouldn't get woken
up. IE you'll get one wakeup.

Thanks,

-Jason

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Hagen Paul Pfeifer]

* Jason Baron | 2012-03-29 10:16:53 [-0400]:

>Right, for level triggered events, they all wait up. However, if you use
>edge triggered, ie add 'EPOLLET', then the event gets 'consumed' by the
>first thread that wakes up, and the subseqent waiters wouldn't get woken
>up. IE you'll get one wakeup.

I addressed level triggered, right - it match the model. But I don't wanted to
wake up every every thread anyway. I don't want to abandon level triggered
functioning.

Any objective against this flag?

Hagen

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Jason Baron]

On Thu, Mar 29, 2012 at 05:05:41PM +0200, Hagen Paul Pfeifer wrote:
> * Jason Baron | 2012-03-29 10:16:53 [-0400]:
> 
> >Right, for level triggered events, they all wait up. However, if you use
> >edge triggered, ie add 'EPOLLET', then the event gets 'consumed' by the
> >first thread that wakes up, and the subseqent waiters wouldn't get woken
> >up. IE you'll get one wakeup.
> 
> I addressed level triggered, right - it match the model. But I don't wanted to
> wake up every every thread anyway. I don't want to abandon level triggered
> functioning.
> 
> Any objective against this flag?
> 

I was trying to better understand the use-case, since at least for the
test case you posted, 'EPOLLET', already does what you want.

Also, the 'EPOLLEXCLUSIVE' flag in your patch addresses multiple threads
blocking on *different* epoll fds. However, if multiple threads are
blocked on a single epoll fd, they will all be woken even if 'EPOLLEXCLUSIVE'
is set. Shouldn't 'EPOLLEXCLUSIVE' affect that case too?

Thanks,

-Jason

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Hagen Paul Pfeifer]

* Jason Baron | 2012-03-29 11:53:24 [-0400]:

>I was trying to better understand the use-case, since at least for the
>test case you posted, 'EPOLLET', already does what you want.
>
>Also, the 'EPOLLEXCLUSIVE' flag in your patch addresses multiple threads
>blocking on *different* epoll fds. However, if multiple threads are
>blocked on a single epoll fd, they will all be woken even if 'EPOLLEXCLUSIVE'
>is set. Shouldn't 'EPOLLEXCLUSIVE' affect that case too?

Hey Jason,

I just wanted to address the "main use-case" (as implemented in a bunch of
network server): one listen socket (say 80) is created and a epoll fd is
created. The listen socket is added to the set and n threads are created
afterwards. So now you have the situation that one listening socket is added
to the set and all threads are awoken if a new client connects. This patch
reduce the useless-all-thread-awoken-overhead by awake only one thread.

Hagen

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Jason Baron]

On Thu, Mar 29, 2012 at 06:32:22PM +0200, Hagen Paul Pfeifer wrote:
> * Jason Baron | 2012-03-29 11:53:24 [-0400]:
> 
> >I was trying to better understand the use-case, since at least for the
> >test case you posted, 'EPOLLET', already does what you want.
> >
> >Also, the 'EPOLLEXCLUSIVE' flag in your patch addresses multiple threads
> >blocking on *different* epoll fds. However, if multiple threads are
> >blocked on a single epoll fd, they will all be woken even if 'EPOLLEXCLUSIVE'
> >is set. Shouldn't 'EPOLLEXCLUSIVE' affect that case too?
> 
> Hey Jason,
> 
> I just wanted to address the "main use-case" (as implemented in a bunch of
> network server): one listen socket (say 80) is created and a epoll fd is
> created. The listen socket is added to the set and n threads are created
> afterwards. So now you have the situation that one listening socket is added
> to the set and all threads are awoken if a new client connects. This patch
> reduce the useless-all-thread-awoken-overhead by awake only one thread.
> 
> Hagen

Hi,

But the behavior of the testcase you've supplied is not changed by the
'EPOLLEXCLUSIVE' support. So is this not the right testcase?

Thanks,

-Jason

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Hagen Paul Pfeifer]

* Jason Baron | 2012-03-29 14:54:17 [-0400]:

>But the behavior of the testcase you've supplied is not changed by the
>'EPOLLEXCLUSIVE' support. So is this not the right testcase?

Hi Jason,

I am currently abroad, the example yesterday was just a quick hack somewhere
in between hotel and meeting to demonstrate the behaviour. ;)

I travel with a company laptop, my testcode is on another PC, I have access to
my git tree and just wanted to get this patch merged in this window.

Hagen

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Andy Lutomirski]

On 03/29/2012 08:05 AM, Hagen Paul Pfeifer wrote:
> * Jason Baron | 2012-03-29 10:16:53 [-0400]:
> 
>> Right, for level triggered events, they all wait up. However, if you use
>> edge triggered, ie add 'EPOLLET', then the event gets 'consumed' by the
>> first thread that wakes up, and the subseqent waiters wouldn't get woken
>> up. IE you'll get one wakeup.
> 
> I addressed level triggered, right - it match the model. But I don't wanted to
> wake up every every thread anyway. I don't want to abandon level triggered
> functioning.

I think that what you want is a mode in which, once epoll_wait returns,
the bits it returns get cleared from the event mask.  Am I right?

(To get this right, you'd need another flag that disables the automatic
addition of POLLERR and POLLHUP.)

If you do that and add an extra syscall that simultaneously does a bunch
of epoll_ctls, a timerfd_settime, and an epoll_wait, I'll be extra happy.

--Andy

Re: [PATCH Resend] epoll: add EPOLLEXCLUSIVE support

[Hagen Paul Pfeifer]

Ping? Any signed-off-by for this patch now? Andrew?