Re: [Bug?] cvs commit: httpd-2.0/server core.c
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
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
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
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