Re: [patch 00/13] Syslets, "Threadlets", generic AIO support, v3

[Ingo Molnar]

* Evgeniy Polyakov <johnpol@xxxxxxxxxxx> wrote:

> > i dont care whether they are separate or not - but you have not 
> > replied to the request that there be a handle_web_request() function 
> > in /both/ files, which is precisely the same function. I didnt ask 
> > you to merge the two files - i only asked for the two web handling 
> > functions to be one and the same function.
> 
> They are not the same in general - if kevent is ready immediately, there 
> will not be its removing from kevent tree, but current kevent server has 
> it always not-immediately for lighttpd tests - so functions are the same:
> open()
> sendfile()
> cork_off
> close(fd)
> close(s)
> remove_event_from_the_kernel
> 
> with the same parameters.

you /STILL/ dont understand. I'm only talking about evserver_epoll.c and 
evserver_kevent.c. Not about lighttpd. Not about historic reasons. I 
simply suggested a common-sense change:

| | Would it be so hard to introduce a single handle_web_request() 
| | function that is exactly the same in the two tests? All the queueing 
| | details (which are of course different in the epoll and the kevent 
| | case) should be in the client function, which calls 
| | handle_web_request().

i.e. put remove_event_from_the_kernel() (kweb_kevent_remove()) and 
evtest_remove()) into a SEPARATE client function, which calls the 
/common/ handle_web_request(sock) function. You can do the 
immediate-removal in that separate, kevent-specific client function - 
but the socket function, handle_web_request(sock) should be /perfectly 
identical/ in the two files.

I.e.:

static inline int handle_web_request(int s)
{
	int err, fd, on = 0;
	off_t offset = 0;
	int count = 40960;
	char path[] = "/tmp/index.html";
	char buf[4096];
		
	err = recv(s, buf, sizeof(buf), 0);
	if (err <= 0)
		return err;

	fd = open(path, O_RDONLY);
	if (fd == -1)
		return fd;

	err = sendfile(s, fd, &offset, count);
	if (err < 0) {
		ulog_err("Failed send %d bytes: fd=%d.\n", count, s);
		close(fd);
		return err;
	}

	setsockopt(s, SOL_TCP, TCP_CORK, &on, sizeof(on));
	close(fd);
	close(s); /* No keepalive */

	return 0;
}

And in evserver_epoll.c do this:

static int evtest_callback_client(int s)
{
	int err = handle_web_request(s);
	if (err)
		evtest_remove(s);
	return err;
}

and in evserver_kevent.c do this:

static int kweb_callback_client(struct ukevent *e, int im)
{
	int err = handle_web_request(e->id.raw[0]);
	if (err || !im)
		kweb_kevent_remove(e);
	return err;
}

ok?

Btw., am i correct that in this particular 'ab' test, the 'immediately' 
flag is always zero, i.e. kweb_kevent_remove() is always called?

	Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/