Re: [Bug?] cvs commit: httpd-2.0/server core.c

2004-01-14 Thread Brad Nicholes
What about the case where they did have sendfile, but did not use
large
file support?  [Did/Do] we attempt to test the EnableSendfile logic?

Yes, that happens in core.c/default_handler() when apr_file_open() is
called.  If the build has send_file support then the
APR_SENDFILE_ENABLED flag is passed to apr_file_open() based on the
ENABLE_SENDFILE_* flag.

Not arguing that breaking up huge responses is a bad thing :) 
However
I'm somewhat confused why apr doesn't handle this gracefully.

Agreed.  There seems to be a lot of code in core.c that is based on the
APR_HAS_SENDFILE flag.  My guess is that at least some of it, if not
all, could be cleaned up.  But then I am not on a platform that has
sendfile() so it is hard for me to tell what is required and what
isn't.

Brad


Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

 William A. Rowe, Jr. [EMAIL PROTECTED] Tuesday, January 13,
2004 7:01:06 PM 
At 07:05 PM 1/13/2004, Brad Nicholes wrote:
I don't think so because the split into multiple bucket code
was
only enabled if both large_file and send_file was enabled.  Which
meant
that on a non-large_file build the check for ENABLE_SENDFILE_OFF
wasn't
there anyway.  If they have large_file support and don't have
send_file
(ie. NetWare), then the file must be split into multiple buckets or
it
doesn't work (32/64 bit type mismatches in the file size).  If they
have
large_file support and send_file, then everything is as it was before.


What about the case where they did have sendfile, but did not use
large
file support?  [Did/Do] we attempt to test the EnableSendfile logic?

If not, perhaps we should.  There are other cases, e.g. some NFS
volume
strategies, where a raw kernel sendfile turns out to be fatal on some
platforms.


I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway. 
This
code doesn't really have anything to do with whether or not sendfile
is
used.  All it does is split a large file into multiple smaller
buckets. 
If later down the line sendfile is used to actually send the file
from
multiple buckets, great.  If not, that is fine also (as demonstrated
by
the fact that NetWare doesn't have sendfile() and it all works fine).

Not arguing that breaking up huge responses is a bad thing :)  However
I'm somewhat confused why apr doesn't handle this gracefully.

Bill  



Re: [Bug?] cvs commit: httpd-2.0/server core.c

2004-01-13 Thread William A. Rowe, Jr.
Woha...

At 11:50 AM 1/8/2004, [EMAIL PROTECTED] wrote:
bnicholes2004/01/08 09:50:03

  Modified:server   core.c
  Log:
  If large file support is enabled allow the file to be split into AP_MAX_SENDFILE 
 sized buckets.  Otherwise Apache will be unable to send files larger than 2 gig due 
 to signed 32-bit limitations.
  
  RCS file: /home/cvs/httpd-2.0/server/core.c,v
  retrieving revision 1.254
  retrieving revision 1.255
  diff -u -r1.254 -r1.255
  --- core.c1 Jan 2004 13:26:23 -   1.254
  +++ core.c8 Jan 2004 17:50:03 -   1.255
  @@ -3508,8 +3508,12 @@
   }
   
   bb = apr_brigade_create(r-pool, c-bucket_alloc);
  -#if APR_HAS_SENDFILE  APR_HAS_LARGE_FILES
  +#if APR_HAS_LARGE_FILES
  +#if APR_HAS_SENDFILE
   if ((d-enable_sendfile != ENABLE_SENDFILE_OFF) 
  +#else
  +if (
  +#endif
   (r-finfo.size  AP_MAX_SENDFILE)) {
   /* APR_HAS_LARGE_FILES issue; must split into mutiple buckets,
* no greater than MAX(apr_size_t), and more granular than that

Ok that is a messy one to grok but I think I got it...

Haven't you broken the EnableSendfile off directive if the user is trying to avoid
using sendfile on non-largefile builds?

E.g. if someone determines that their sendfile implementation is broken, will
the server still react properly to EnableSendfile off on linux or bsd?

Bill




Re: [Bug?] cvs commit: httpd-2.0/server core.c

2004-01-13 Thread Brad Nicholes
I don't think so because the split into multiple bucket code was
only enabled if both large_file and send_file was enabled.  Which meant
that on a non-large_file build the check for ENABLE_SENDFILE_OFF wasn't
there anyway.  If they have large_file support and don't have send_file
(ie. NetWare), then the file must be split into multiple buckets or it
doesn't work (32/64 bit type mismatches in the file size).  If they have
large_file support and send_file, then everything is as it was before. 
I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway.  This
code doesn't really have anything to do with whether or not sendfile is
used.  All it does is split a large file into multiple smaller buckets. 
If later down the line sendfile is used to actually send the file from
multiple buckets, great.  If not, that is fine also (as demonstrated by
the fact that NetWare doesn't have sendfile() and it all works fine).

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

 [EMAIL PROTECTED] Tuesday, January 13, 2004 4:04:30 PM 
Woha...

At 11:50 AM 1/8/2004, [EMAIL PROTECTED] wrote:
bnicholes2004/01/08 09:50:03

  Modified:server   core.c
  Log:
  If large file support is enabled allow the file to be split into
AP_MAX_SENDFILE sized buckets.  Otherwise Apache will be unable to send
files larger than 2 gig due to signed 32-bit limitations.
  
  RCS file: /home/cvs/httpd-2.0/server/core.c,v
  retrieving revision 1.254
  retrieving revision 1.255
  diff -u -r1.254 -r1.255
  --- core.c1 Jan 2004 13:26:23 -   1.254
  +++ core.c8 Jan 2004 17:50:03 -   1.255
  @@ -3508,8 +3508,12 @@
   }
   
   bb = apr_brigade_create(r-pool, c-bucket_alloc);
  -#if APR_HAS_SENDFILE  APR_HAS_LARGE_FILES
  +#if APR_HAS_LARGE_FILES
  +#if APR_HAS_SENDFILE
   if ((d-enable_sendfile != ENABLE_SENDFILE_OFF) 
  +#else
  +if (
  +#endif
   (r-finfo.size  AP_MAX_SENDFILE)) {
   /* APR_HAS_LARGE_FILES issue; must split into mutiple
buckets,
* no greater than MAX(apr_size_t), and more granular
than that

Ok that is a messy one to grok but I think I got it...

Haven't you broken the EnableSendfile off directive if the user is
trying to avoid
using sendfile on non-largefile builds?

E.g. if someone determines that their sendfile implementation is
broken, will
the server still react properly to EnableSendfile off on linux or bsd?

Bill




Re: [Bug?] cvs commit: httpd-2.0/server core.c

2004-01-13 Thread William A. Rowe, Jr.
At 07:05 PM 1/13/2004, Brad Nicholes wrote:
I don't think so because the split into multiple bucket code was
only enabled if both large_file and send_file was enabled.  Which meant
that on a non-large_file build the check for ENABLE_SENDFILE_OFF wasn't
there anyway.  If they have large_file support and don't have send_file
(ie. NetWare), then the file must be split into multiple buckets or it
doesn't work (32/64 bit type mismatches in the file size).  If they have
large_file support and send_file, then everything is as it was before. 

What about the case where they did have sendfile, but did not use large
file support?  [Did/Do] we attempt to test the EnableSendfile logic?

If not, perhaps we should.  There are other cases, e.g. some NFS volume
strategies, where a raw kernel sendfile turns out to be fatal on some platforms.


I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway.  This
code doesn't really have anything to do with whether or not sendfile is
used.  All it does is split a large file into multiple smaller buckets. 
If later down the line sendfile is used to actually send the file from
multiple buckets, great.  If not, that is fine also (as demonstrated by
the fact that NetWare doesn't have sendfile() and it all works fine).

Not arguing that breaking up huge responses is a bad thing :)  However
I'm somewhat confused why apr doesn't handle this gracefully.

Bill