Re: dd PATCH: add conv=direct

[Andy Isaacson]

On Tue, Apr 06, 2004 at 05:33:26PM -0700, Andrew Morton wrote:
> Andy Isaacson <address@hidden> wrote:
> >          dd(1) is convenient for this purpose, but is lacking a method
> > to force O_DIRECT.  The enclosed patch adds a "conv=direct" flag to
> > enable this usage.
> 
> This would be rather nice to have.  You'll need to ensure that the data
> is page-aligned in memory.

So, some confusion on my part about O_DIRECT:  I can't get O_DIRECT to
work on ext3, at all, on 2.4.25 -- open(O_DIRECT) succeeds, but the write
returns EINVAL.  Same code works fine when writing to a block device.
If the problem is that ext3 can't support O_DIRECT, why does the open
succeed?

> While you're there, please add an fsync-before-closing option.

Easy enough.  How does this look?  Note that C_TWOBUFS ensures the
output buffer is getpagesize()-aligned.

The next feature to add would be OpenBSD-style "KB/s" reporting.  I'm
not going there.

-andy

2004-04-07  Andy Isaacson  <address@hidden>

        * add conv=direct and conv=fsync options.

diff -ur coreutils-5.0.91/doc/coreutils.texi 
coreutils-5.0.91-adi/doc/coreutils.texi
--- coreutils-5.0.91/doc/coreutils.texi 2003-09-04 16:26:51.000000000 -0500
+++ coreutils-5.0.91-adi/doc/coreutils.texi     2004-04-07 11:26:36.000000000 
-0500
@@ -6373,6 +6373,16 @@
 Pad every input block to size of @samp{ibs} with trailing zero bytes.
 When used with @samp{block} or @samp{unblock}, pad with spaces instead of
 zero bytes.
+
address@hidden fsync
address@hidden fsync
+Call @samp{fsync(2)} on the output file before exiting.  This ensures
+that the file data is written to permanent store.
+
address@hidden direct
address@hidden direct
+Open the output file with O_DIRECT, avoiding (on Linux) using the buffer
+cache.
 @end table
 
 @end table
diff -ur coreutils-5.0.91/src/dd.c coreutils-5.0.91-adi/src/dd.c
--- coreutils-5.0.91/src/dd.c   2003-07-25 02:43:09.000000000 -0500
+++ coreutils-5.0.91-adi/src/dd.c       2004-04-07 11:45:23.000000000 -0500
@@ -66,6 +66,9 @@
 /* Default input and output blocksize. */
 #define DEFAULT_BLOCKSIZE 512
 
+/* XXX hack */
+#define HAVE_O_DIRECT 1
+
 /* Conversions bit masks. */
 #define C_ASCII 01
 #define C_EBCDIC 02
@@ -78,6 +81,8 @@
 #define C_NOERROR 0400
 #define C_NOTRUNC 01000
 #define C_SYNC 02000
+#define C_FSYNC 010000
+#define C_DIRECT 020000
 /* Use separate input and output buffers, and combine partial input blocks. */
 #define C_TWOBUFS 04000
 
@@ -162,6 +167,10 @@
   {"noerror", C_NOERROR},      /* Ignore i/o errors. */
   {"notrunc", C_NOTRUNC},      /* Do not truncate output file. */
   {"sync", C_SYNC},            /* Pad input records to ibs with NULs. */
+  {"fsync", C_FSYNC},          /* call fsync(2) before closing output file */
+#ifdef HAVE_O_DIRECT
+  {"direct", C_DIRECT | C_TWOBUFS},    /* open files with O_DIRECT */
+#endif
   {NULL, 0}
 };
 
@@ -1190,6 +1199,11 @@
        = (O_CREAT
           | (seek_records || (conversions_mask & C_NOTRUNC) ? 0 : O_TRUNC));
 
+#if HAVE_O_DIRECT
+      if (conversions_mask & C_DIRECT)
+       opts |= O_DIRECT;
+#endif
+
       /* Open the output file with *read* access only if we might
         need to read to satisfy a `seek=' request.  If we can't read
         the file, go ahead with write-only access; it might work.  */
@@ -1240,5 +1254,11 @@
 
   exit_status = dd_copy ();
 
+  if (conversions_mask & C_FSYNC)
+    {
+      if (fsync (STDOUT_FILENO) != 0)
+       error(0, errno, _("cannot fsync %s"), quote (output_file));
+    }
+
   quit (exit_status);
 }