Re: dd PATCH: add conv=direct

[Andy Isaacson]

On Wed, Apr 07, 2004 at 01:03:08PM -0700, Andrew Morton wrote:
> Andy Isaacson <address@hidden> wrote:
> > I claim that O_DIRECT on of= is important because you just plain *can
> > not* do the minimal-sized IDE block scrub without it.  I don't yet see a
> > similar benefit to O_DIRECT on if= side.
> 
> If you want a block scrubber then write a block scrubber.

They exist.  They're a pain in the ass to find when you need one.  dd is
almost there; a small patch that might have a chance of getting accepted
upstream seemed like a reasonable time investment.

> If you want to add O_DIRECT support to dd then it should be implemented
> properly, and that means implementing it for both read and write.
> 
> In fact the user should be able to specify the read-O_DIRECT and the
> write-O_DIRECT independently - if for no other reason than that the source
> and dest filesytems may not both support O_DIRECT.

Of course if both directions are supported they must be independently
specifiable.  I just don't see a compelling use case for the input side.

Nevertheless, here you go.  Still needs some autoconfiscation.

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

        * add conv=direct, conv=idirect, and conv=fsync options.

diff -u'rx*.1' 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 15:24:17.000000000 
-0500
@@ -6373,6 +6373,20 @@
 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.
+
address@hidden idirect
address@hidden idirect
+Open the input file with O_DIRECT, avoiding (on Linux) using the buffer
 @end table
 
 @end table
diff -u'rx*.1' 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 15:23:24.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,9 @@
 #define C_NOERROR 0400
 #define C_NOTRUNC 01000
 #define C_SYNC 02000
+#define C_FSYNC 010000
+#define C_DIRECT 020000
+#define C_IDIRECT 040000
 /* Use separate input and output buffers, and combine partial input blocks. */
 #define C_TWOBUFS 04000
 
@@ -162,6 +168,11 @@
   {"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 output with O_DIRECT */
+  {"idirect", C_IDIRECT | C_TWOBUFS},  /* open input with O_DIRECT */
+#endif
   {NULL, 0}
 };
 
@@ -1177,7 +1188,14 @@
 
   if (input_file != NULL)
     {
-      if (open_fd (STDIN_FILENO, input_file, O_RDONLY, 0) < 0)
+      int opts = O_RDONLY;
+
+#if HAVE_O_DIRECT
+      if (conversions_mask & C_IDIRECT)
+       opts |= O_DIRECT;
+#endif
+
+      if (open_fd (STDIN_FILENO, input_file, opts, 0) < 0)
        error (EXIT_FAILURE, errno, _("opening %s"), quote (input_file));
     }
   else
@@ -1190,6 +1208,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 +1263,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);
 }