On Wed, Nov 26, 2014 at 4:06 PM, Denys Vlasenko
<[email protected]> wrote:
> Looks like it will always allocate the copy buffer,
> and always attempt the final read, even if sendfile worked perfectly.
> Can this be avoided?
Please review attached patch.
diff -d -urpN busybox.8/Config.in busybox.9/Config.in
--- busybox.8/Config.in 2014-08-15 13:04:28.000000000 +0200
+++ busybox.9/Config.in 2014-11-26 16:32:30.791687946 +0100
@@ -264,6 +264,13 @@ config PAM
Use PAM in some busybox applets (currently login and httpd) instead
of direct access to password database.
+config FEATURE_USE_SENDFILE
+ bool "Use sendfile system call"
+ default y
+ help
+ When enabled, busybox will use the kernel sendfile() function
+ instead of read/write loops where applicable.
+
config LONG_OPTS
bool "Support for --long-options"
default y
diff -d -urpN busybox.8/libbb/copyfd.c busybox.9/libbb/copyfd.c
--- busybox.8/libbb/copyfd.c 2014-08-15 13:04:28.000000000 +0200
+++ busybox.9/libbb/copyfd.c 2014-11-26 17:38:22.814181931 +0100
@@ -8,6 +8,9 @@
*/
#include "libbb.h"
+#if ENABLE_FEATURE_USE_SENDFILE
+# include <sys/sendfile.h>
+#endif
/* Used by NOFORK applets (e.g. cat) - must not use xmalloc.
* size < 0 means "ignore write errors", used by tar --to-command
@@ -18,12 +21,13 @@ static off_t bb_full_fd_action(int src_f
int status = -1;
off_t total = 0;
bool continue_on_write_error = 0;
+ ssize_t sendfile_sz;
#if CONFIG_FEATURE_COPYBUF_KB <= 4
char buffer[CONFIG_FEATURE_COPYBUF_KB * 1024];
enum { buffer_size = sizeof(buffer) };
#else
- char *buffer;
- int buffer_size;
+ char *buffer = buffer; /* for compiler */
+ int buffer_size = 0;
#endif
if (size < 0) {
@@ -31,46 +35,58 @@ static off_t bb_full_fd_action(int src_f
continue_on_write_error = 1;
}
-#if CONFIG_FEATURE_COPYBUF_KB > 4
- if (size > 0 && size <= 4 * 1024)
- goto use_small_buf;
- /* We want page-aligned buffer, just in case kernel is clever
- * and can do page-aligned io more efficiently */
- buffer = mmap(NULL, CONFIG_FEATURE_COPYBUF_KB * 1024,
- PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANON,
- /* ignored: */ -1, 0);
- buffer_size = CONFIG_FEATURE_COPYBUF_KB * 1024;
- if (buffer == MAP_FAILED) {
- use_small_buf:
- buffer = alloca(4 * 1024);
- buffer_size = 4 * 1024;
- }
-#endif
-
if (src_fd < 0)
goto out;
+ sendfile_sz = !ENABLE_FEATURE_USE_SENDFILE
+ ? 0
+ : MAXINT(ssize_t) - 0xffff;
if (!size) {
- size = buffer_size;
+ size = MAXINT(ssize_t) - 0xffff;
status = 1; /* copy until eof */
}
while (1) {
ssize_t rd;
- rd = safe_read(src_fd, buffer, size > buffer_size ? buffer_size : size);
-
- if (!rd) { /* eof - all done */
- status = 0;
- break;
+ if (sendfile_sz) {
+#if ENABLE_FEATURE_USE_SENDFILE
+ rd = sendfile(dst_fd, src_fd, NULL,
+ size > sendfile_sz ? sendfile_sz : size);
+ if (rd >= 0)
+ goto read_ok;
+#endif
+ sendfile_sz = 0; /* do not try sendfile anymore */
}
+ if (CONFIG_FEATURE_COPYBUF_KB > 4 && buffer_size == 0) {
+ if (size > 0 && size <= 4 * 1024)
+ goto use_small_buf;
+ /* We want page-aligned buffer, just in case kernel is clever
+ * and can do page-aligned io more efficiently */
+ buffer = mmap(NULL, CONFIG_FEATURE_COPYBUF_KB * 1024,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON,
+ /* ignored: */ -1, 0);
+ buffer_size = CONFIG_FEATURE_COPYBUF_KB * 1024;
+ if (buffer == MAP_FAILED) {
+ use_small_buf:
+ buffer = alloca(4 * 1024);
+ buffer_size = 4 * 1024;
+ }
+ }
+ rd = safe_read(src_fd, buffer,
+ size > buffer_size ? buffer_size : size);
if (rd < 0) {
bb_perror_msg(bb_msg_read_error);
break;
}
+ read_ok:
+ if (!rd) { /* eof - all done */
+ status = 0;
+ break;
+ }
/* dst_fd == -1 is a fake, else... */
- if (dst_fd >= 0) {
+ if (dst_fd >= 0 && !sendfile_sz) {
ssize_t wr = full_write(dst_fd, buffer, rd);
if (wr < rd) {
if (!continue_on_write_error) {
@@ -93,7 +109,7 @@ static off_t bb_full_fd_action(int src_f
out:
#if CONFIG_FEATURE_COPYBUF_KB > 4
- if (buffer_size != 4 * 1024)
+ if (buffer_size > 4 * 1024)
munmap(buffer, buffer_size);
#endif
return status ? -1 : total;
diff -d -urpN busybox.8/networking/Config.src busybox.9/networking/Config.src
--- busybox.8/networking/Config.src 2014-08-15 13:04:28.000000000 +0200
+++ busybox.9/networking/Config.src 2014-11-26 16:32:30.792687946 +0100
@@ -181,14 +181,6 @@ config FEATURE_HTTPD_RANGES
"Range: bytes=NNN-[MMM]" header. Allows for resuming interrupted
downloads, seeking in multimedia players etc.
-config FEATURE_HTTPD_USE_SENDFILE
- bool "Use sendfile system call"
- default y
- depends on HTTPD
- help
- When enabled, httpd will use the kernel sendfile() function
- instead of read/write loop.
-
config FEATURE_HTTPD_SETUID
bool "Enable -u <user> option"
default y
diff -d -urpN busybox.8/networking/httpd.c busybox.9/networking/httpd.c
--- busybox.8/networking/httpd.c 2014-08-15 13:04:28.000000000 +0200
+++ busybox.9/networking/httpd.c 2014-11-26 16:32:30.794687947 +0100
@@ -133,7 +133,7 @@
# include <security/pam_appl.h>
# include <security/pam_misc.h>
#endif
-#if ENABLE_FEATURE_HTTPD_USE_SENDFILE
+#if ENABLE_FEATURE_USE_SENDFILE
# include <sys/sendfile.h>
#endif
/* amount of buffering in a pipe */
@@ -1624,7 +1624,7 @@ static NOINLINE void send_file_and_exit(
#endif
if (what & SEND_HEADERS)
send_headers(HTTP_OK);
-#if ENABLE_FEATURE_HTTPD_USE_SENDFILE
+#if ENABLE_FEATURE_USE_SENDFILE
{
off_t offset = range_start;
while (1) {
@@ -1654,7 +1654,7 @@ static NOINLINE void send_file_and_exit(
break;
}
if (count < 0) {
- IF_FEATURE_HTTPD_USE_SENDFILE(fin:)
+ IF_FEATURE_USE_SENDFILE(fin:)
if (verbose > 1)
bb_perror_msg("error");
}
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
