You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ryan Bloom <rb...@ntrnet.net> on 2000/12/28 07:46:58 UTC
[PATCH] Sendfile on old FreeBSD's
Okay, so this patch doesn't make much sense, but it solves a problem I am
having on a FreeBSD 4.1 machine. Basically, sendfile on my 4.2 FreeBSD
machine returns EAGAIN, and the number of bytes sent, including bytes from
the headers. FreeBSD 4.1 however, returns 0 (success) and the number of
bytes sent, minus the bytes sent from the headers. I didn't dig into the
FreeBSD code too much, because it's late and I just needed something that
works minimally right now. Hopefully I'll have time to dig into the code
in more detail tomorrow. The little bit that I did investigate looks
identical between 4.1 and 4.2, which could mean that there is more going
on than we are currently aware of.
Anyway, could somebody with more in-depth knowledge of FreeBSD look at
this, and see if this patch makes sense? The other option is to just turn
off sendfile support in FreeBSD for anything other than 4.2.
Ryan
Index: modules/http/http_core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
retrieving revision 1.231
diff -u -r1.231 http_core.c
--- modules/http/http_core.c 2000/12/22 23:43:16 1.231
+++ modules/http/http_core.c 2000/12/28 06:36:52
@@ -2605,7 +2605,8 @@
rv = apr_sendfile(c->client_socket, fd, hdtr, &file_offset, &tmplen,
flags);
total_bytes_left -= tmplen;
- if (!total_bytes_left || (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv))) {
+ if (!total_bytes_left || rv == APR_SUCCESS ||
+ (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv))) {
return rv; /* normal case & error exit */
}
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] Sendfile on old FreeBSD's
Posted by Tony Finch <do...@dotat.at>.
Ryan Bloom <rb...@ntrnet.net> wrote:
>
>Okay, so this patch doesn't make much sense, but it solves a problem I am
>having on a FreeBSD 4.1 machine. Basically, sendfile on my 4.2 FreeBSD
>machine returns EAGAIN, and the number of bytes sent, including bytes from
>the headers. FreeBSD 4.1 however, returns 0 (success) and the number of
>bytes sent, minus the bytes sent from the headers.
That code hasn't changed at all so the difference in behaviour must be
caused by something else.
>Anyway, could somebody with more in-depth knowledge of FreeBSD look at
>this, and see if this patch makes sense?
Looks good.
Tony.
--
f.a.n.finch fanf@covalent.net dot@dotat.at
"You realize there's a government directive stating
that there is no such thing as a flying saucer?"
Re: [PATCH] Sendfile on old FreeBSD's
Posted by rb...@covalent.net.
> . looking at BSD 4.1 sendfile(), it seems clear that the number of
> bytes sent includes bytes sent from the header (unless the
> kernel writev() is broken); the 4.2 logic for bytes sent from the
> header looks the same; the kernel code doesn't seem to match your
> empirical results; I guess you had some trace points in
> apr_sendfile() to know how it was called (parms updated properly?);
> sorry for the questioning but it is hard to verify your
> interpretation without detailed information :)
Everything was called properly. I have two machines, one with 4.1 and one
with 4.2. I stepped through both machines and got different results
back. It almost looks like the bytes_sent field is treated differently
based on the sendfile() return code, but that would be really stupid.
> . If FreeBSD's sendfile can hit EAGAIN on the body after having sent
> bytes from the header, shouldn't we return APR_SUCCESS?
Maybe. I am following the general rule that we return the same response
that the OS returns.
> apr_sendfile() results for a socket with APR timeout set:
>
> a real error occurred (not EAGAIN):
> rv set as appropriate, bytes_sent could include a previous entity
>
> sent any data and no real error occurred:
>
> rv APR_SUCCESS, bytes_sent updated
>
> sent no data from header or body or trailer within timeout period:
>
> rv APR_TIMEUP, bytes_sent could include previous entity
>
> I see no room for APR_EAGAIN. That isn't something the app should
> have to handle if an APR timeout is set.
This makes sense to me. I'll look at modifying apr_sendfile on FreeBSD to
actually do this properly.
> > Anyway, could somebody with more in-depth knowledge of FreeBSD look at
> > this, and see if this patch makes sense? The other option is to just turn
> > off sendfile support in FreeBSD for anything other than 4.2.
>
> Does test/sendfile.c ever find a problem on any FreeBSD level?
It works just fine on 4.2, but fails miserably on 4.1. I am beginning to
think that FreeBSD's sendfile implementation may have some problems.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] Sendfile on old FreeBSD's
Posted by Jeff Trawick <tr...@bellsouth.net>.
Ryan Bloom <rb...@ntrnet.net> writes:
> Okay, so this patch doesn't make much sense, but it solves a problem I am
> having on a FreeBSD 4.1 machine. Basically, sendfile on my 4.2 FreeBSD
> machine returns EAGAIN, and the number of bytes sent, including bytes from
> the headers. FreeBSD 4.1 however, returns 0 (success) and the number of
> bytes sent, minus the bytes sent from the headers. I didn't dig into the
> FreeBSD code too much, because it's late and I just needed something that
> works minimally right now. Hopefully I'll have time to dig into the code
> in more detail tomorrow. The little bit that I did investigate looks
> identical between 4.1 and 4.2, which could mean that there is more going
> on than we are currently aware of.
A couple of points from an ignorant observer...
. looking at BSD 4.1 sendfile(), it seems clear that the number of
bytes sent includes bytes sent from the header (unless the
kernel writev() is broken); the 4.2 logic for bytes sent from the
header looks the same; the kernel code doesn't seem to match your
empirical results; I guess you had some trace points in
apr_sendfile() to know how it was called (parms updated properly?);
sorry for the questioning but it is hard to verify your
interpretation without detailed information :)
. If FreeBSD's sendfile can hit EAGAIN on the body after having sent
bytes from the header, shouldn't we return APR_SUCCESS?
I'm a bit uncomfortable (ignorant?) with the last couple of changes to
the apr_sendfile() status checking in http_core.c. It seems that it
is getting a bit tricky for the app and that some of the statuses will
only be returned with certain versions of a certain operating system.
This makes it extremely likely that apps will be broken.
Shouldn't we hide some of this system-dependent stuff in
apr_sendfile() on behalf of the app?
apr_sendfile() results for a socket with APR timeout set:
a real error occurred (not EAGAIN):
rv set as appropriate, bytes_sent could include a previous entity
sent any data and no real error occurred:
rv APR_SUCCESS, bytes_sent updated
sent no data from header or body or trailer within timeout period:
rv APR_TIMEUP, bytes_sent could include previous entity
I see no room for APR_EAGAIN. That isn't something the app should
have to handle if an APR timeout is set.
> Anyway, could somebody with more in-depth knowledge of FreeBSD look at
> this, and see if this patch makes sense? The other option is to just turn
> off sendfile support in FreeBSD for anything other than 4.2.
Does test/sendfile.c ever find a problem on any FreeBSD level?
>
> Ryan
>
> Index: modules/http/http_core.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
> retrieving revision 1.231
> diff -u -r1.231 http_core.c
> --- modules/http/http_core.c 2000/12/22 23:43:16 1.231
> +++ modules/http/http_core.c 2000/12/28 06:36:52
> @@ -2605,7 +2605,8 @@
> rv = apr_sendfile(c->client_socket, fd, hdtr, &file_offset, &tmplen,
> flags);
> total_bytes_left -= tmplen;
> - if (!total_bytes_left || (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv))) {
> + if (!total_bytes_left || rv == APR_SUCCESS ||
> + (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv))) {
> return rv; /* normal case & error exit */
> }
--
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...