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

Ingo Molnar
2007-03-01T13:19:25Z

* Evgeniy Polyakov <johnpol@2ka.mipt.ru> 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