Re: [PATCH] dd: work around buffer length restrictions with oflag=direct (O_DIRECT)
Eric Sandeen wrote: Jim Meyering wrote: Eric Sandeen mentioned that dd's O_DIRECT-exposing code didn't always work. The problem is that the kernel imposes draconian restrictions on the size of buffer that one may write to an FD opened with O_DIRECT. Currently, at least on ext4 and with a recent linux kernel, that buffer size must be a multiple of 512, which happens to be dd's default buffer size. (there are alignment restrictions too, just FWIW) ... Yes, we should be ok on that front, with page-aligned buffers. Note also that the code makes no attempt to determine the appropriate block size. So if you chose obs=42, you lose. The adventurous user who experiments with oflags=direct must also select a block size that works with O_DIRECT and the destination file system. I think that's fine, docs can say subject to size and alignment requirements of the kernel and filesystem or something I think. I mentioned size, because that can (and may have to) be changed via the obs, ibs, bs options. I didn't want to mention alignment because dd users can't influence that. ... I suppose it would be nice to at least make that last buffered IO synchronous if possible, or sync the file afterwards. POSIX_FADV_DONTNEED for extra credit? :) Good idea. What do you think of this? It feels slightly excessive to make 3 additional syscalls just to preserve some semantics of O_DIRECT for that final buffer. ...but I like it. From ff159a605e5bc10fe871109f66cba5ee410c9138 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 7 Aug 2009 16:21:35 +0200 Subject: [PATCH] dd: preserve semantics of O_DIRECT even for final block * src/dd.c: Include ignore-value.h (iwrite): When disabling O_DIRECT, try to compensate via POSIX_FADV_DONTNEED and fsync. Suggested by Eric Sandeen. --- src/dd.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/src/dd.c b/src/dd.c index 43ad718..d9e4c85 100644 --- a/src/dd.c +++ b/src/dd.c @@ -29,6 +29,7 @@ #include fd-reopen.h #include gethrxtime.h #include human.h +#include ignore-value.h #include long-options.h #include quote.h #include quotearg.h @@ -843,6 +844,22 @@ iwrite (int fd, char const *buf, size_t size) if (fcntl (STDOUT_FILENO, F_SETFL, old_flags ~O_DIRECT) != 0) error (0, errno, _(failed to turn off O_DIRECT: %s), quote (output_file)); + + /* Since we have just turned off O_DIRECT for the final write, + here we try to preserve some of its semantics. First, use + posix_fadvise to tell the system not to pollute the buffer + cache with this data. Don't bother to diagnose lseek or + posix_fadvise failure. */ +#ifdef POSIX_FADV_DONTNEED + off_t off = lseek (STDOUT_FILENO, 0, SEEK_CUR); + if (0 = off) +ignore_value (posix_fadvise (STDOUT_FILENO, + off, 0, POSIX_FADV_DONTNEED)); +#endif + + /* Attempt to ensure that that final block is committed + to disk as quickly as possible. */ + conversions_mask |= C_FSYNC; } while (total_written size) -- 1.6.4.115.g33d49
Re: [PATCH] dd: work around buffer length restrictions with oflag=direct (O_DIRECT)
Jim Meyering wrote: Eric Sandeen wrote: ... I suppose it would be nice to at least make that last buffered IO synchronous if possible, or sync the file afterwards. POSIX_FADV_DONTNEED for extra credit? :) Good idea. What do you think of this? It feels slightly excessive to make 3 additional syscalls just to preserve some semantics of O_DIRECT for that final buffer. ...but I like it. heh. I like it too. Thanks! From ff159a605e5bc10fe871109f66cba5ee410c9138 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 7 Aug 2009 16:21:35 +0200 Subject: [PATCH] dd: preserve semantics of O_DIRECT even for final block * src/dd.c: Include ignore-value.h (iwrite): When disabling O_DIRECT, try to compensate via POSIX_FADV_DONTNEED and fsync. Suggested by Eric Sandeen. --- src/dd.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/src/dd.c b/src/dd.c index 43ad718..d9e4c85 100644 --- a/src/dd.c +++ b/src/dd.c @@ -29,6 +29,7 @@ #include fd-reopen.h #include gethrxtime.h #include human.h +#include ignore-value.h #include long-options.h #include quote.h #include quotearg.h @@ -843,6 +844,22 @@ iwrite (int fd, char const *buf, size_t size) if (fcntl (STDOUT_FILENO, F_SETFL, old_flags ~O_DIRECT) != 0) error (0, errno, _(failed to turn off O_DIRECT: %s), quote (output_file)); + + /* Since we have just turned off O_DIRECT for the final write, + here we try to preserve some of its semantics. First, use + posix_fadvise to tell the system not to pollute the buffer + cache with this data. Don't bother to diagnose lseek or + posix_fadvise failure. */ +#ifdef POSIX_FADV_DONTNEED + off_t off = lseek (STDOUT_FILENO, 0, SEEK_CUR); + if (0 = off) +ignore_value (posix_fadvise (STDOUT_FILENO, + off, 0, POSIX_FADV_DONTNEED)); +#endif + + /* Attempt to ensure that that final block is committed + to disk as quickly as possible. */ + conversions_mask |= C_FSYNC; } while (total_written size) -- 1.6.4.115.g33d49
Re: BTRFS file clone support for cp
Giuseppe Scrivano wrote:
Jim Meyering j...@meyering.net writes:
+ if (clone_file (dest_desc, source_desc))
+{
+ error (0, errno, _(cannot fstat %s), quote (dst_name));
I prefer this diagnostic ;-)
error (0, errno, _(failed to clone %s), quote (dst_name));
Too wild copypaste :)
I included your notes in the following patch.
Thanks. I think this is finally ready.
Here are a few more small changes:
(alphabetize and change one-line description)
diff --git a/src/cp.c b/src/cp.c
index 635c7c7..ae1c1ce 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -192,11 +192,11 @@ Mandatory arguments to long options are mandatory for
short options too.\n\
), stdout);
fputs (_(\
-R, -r, --recursive copy directories recursively\n\
+ --reflinkperform a lightweight (CoW/clone) copy\n\
--remove-destination remove each existing destination file before\n\
attempting to open it (contrast with
--force)\n\
), stdout);
fputs (_(\
- --reflinkclone the file if it is possible\n\
--sparse=WHENcontrol creation of sparse files\n\
--strip-trailing-slashes remove any trailing slashes from each SOURCE\n\
argument\n\
I did the same with the addition of REFLINK_OPTION here:
enum
{
COPY_CONTENTS_OPTION = CHAR_MAX + 1,
NO_PRESERVE_ATTRIBUTES_OPTION,
PARENTS_OPTION,
PRESERVE_ATTRIBUTES_OPTION,
REFLINK_OPTION,
SPARSE_OPTION,
STRIP_TRAILING_SLASHES_OPTION,
UNLINK_DEST_BEFORE_OPENING
};
Plus, I adjusted the log message:
From a1d7469835371ded0ad8e3496bc5a5bebf94ccef Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 1 Aug 2009 19:36:48 +0200
Subject: [PATCH] cp: accept the --reflink option
* NEWS: Mention it.
* doc/coreutils.texi (cp invocation): Describe it.
* src/copy.h (struct cp_options) [reflink]: New member.
* src/copy.c (usage): Describe it.
(copy_reg): If reflink is true try to clone the file.
(main): Check for --reflink.
(cp_option_init): Initialize the new member.
* src/install.c (cp_option_init): Initialize the new member.
* src/mv.c (cp_option_init): Likewise.
* tests/cp/sparse: Add a new test case.
---
NEWS |4
doc/coreutils.texi |9 +
src/copy.c | 16 ++--
src/copy.h |3 +++
src/cp.c | 14 ++
src/install.c |1 +
src/mv.c |1 +
tests/cp/sparse|4
8 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/NEWS b/NEWS
index c61f666..6df0d65 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,10 @@ GNU coreutils NEWS-*-
outline -*-
chroot now accepts the options --userspec and --groups.
+ cp accepts a new option, --reflink: create a lightweight copy
+ using copy-on-write (COW). This is currently supported only on
+ btrfs file systems.
+
cp now preserves time stamps on symbolic links, when possible
cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 90a54b2..bb1d87a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7543,6 +7543,15 @@ cp invocation
unless you also specify @option{-P}, as @acronym{POSIX} allows
implementations that dereference symbolic links by default.
+...@item --reflink
+...@opindex --reflink
+Perform a lightweight, copy-on-write (COW) copy.
+Copying with this option can succeed only on some relatively new file systems.
+Once it has succeeded, beware that the source and destination files
+share the same disk data blocks as long as they remain unmodified.
+Thus, if a disk I/O error affects data blocks of one of the files,
+the other suffers the exact same fate.
+
@item --remove-destination
@opindex --remove-destination
Remove each existing destination file before attempting to open it
diff --git a/src/copy.c b/src/copy.c
index 24b5f6b..bed90c4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -624,13 +624,16 @@ copy_reg (char const *src_name, char const *dst_name,
goto close_src_and_dst_desc;
}
- /* If --sparse=auto is in effect, attempt a btrfs clone operation.
- If the operation is not supported or it fails then copy the file
- in the usual way. */
- bool copied = (x-sparse_mode == SPARSE_AUTO
- clone_file (dest_desc, source_desc) == 0);
+ if (x-reflink)
+{
+ if (clone_file (dest_desc, source_desc))
+{
+ error (0, errno, _(failed to clone %s), quote (dst_name));
+ return_val = false;
+}
+ goto close_src_and_dst_desc;
+}
- if (!copied)
{
typedef uintptr_t word;
off_t n_read_total = 0;
@@ -2232,6 +2235,7 @@ valid_options (const struct cp_options *co)
assert (VALID_BACKUP_TYPE (co-backup_type));
assert (VALID_SPARSE_MODE (co-sparse_mode));
assert (!(co-hard_link
Report a bug in the ln command
Hello, in the already existing directory /srv/backup/home I wanted to create a link for the user eddi to eddis home directory for data backup reasons. In order not to change into the target directory I used the following command: ln -sf /home/eddi /srv/backup/home/eddi The link eddi was created as expected -in /srv/backup/home. However, under /home/eddi was created a link eddi, too, which refers to /home/eddi. I think that the link in the home directory is a bug. Unfortunately this bug lead in my case to a recursive backup of the home directory. By the way, this phenomenon only occurs by utilizing the force-option f. Best regards Kai Laemmerhit
