Pádraig Brady wrote: ... > I'm going to use this "2.6.38" check to only enable FIEMAP_FLAG_SYNC > before Linux kernel 2.6.38. It's always worth avoiding sync if possible. > Proposed patch attached. > I'll submit my 3 outstanding fiemap patches tomorrow. ... > Subject: [PATCH] copy: with fiemap copy, only sync when needed > > * src/extent-scan.h (struct extent_scan): Add the fm_flags member to > pass to the fiemap scan. > * src/extent-scan.c (extent_need_sync): A new function used to > detect Linux kernels before 2.6.38. > (extent_scan_init): Add FIEMAP_FLAG_SYNC when needed. > * tests/cp/sparse-fiemap: Adjust comment. > * NEWS: Mention the change in behavior. > Indirectly suggested by Mike Frysinger > --- > NEWS | 4 ++++ > src/extent-scan.c | 32 +++++++++++++++++++++++++++++++- > src/extent-scan.h | 3 +++ > tests/cp/sparse-fiemap | 4 ++-- > 4 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/NEWS b/NEWS > index 9c4a16f..d1020cb 100644 > --- a/NEWS > +++ b/NEWS > @@ -31,6 +31,10 @@ GNU coreutils NEWS -*- > outline -*- > bytes from the input, and also can efficiently create a hole in the > output file when --sparse=always is specified. > > + cp now avoids syncing files when possible, when doing a FIEMAP copy. > + The sync in only needed on Linux kernels before 2.6.38. > + [The sync was introduced in coreutils-8.10]
Thanks. This looks good. Minor suggestions below: > diff --git a/src/extent-scan.c b/src/extent-scan.c ... > +/* Work around Linux kernel issues on BTRFS and EXT4 before 2.6.38. */ Please add a comment here, e.g., /* FIXME: remove in 2013, or whenever we're pretty confident that the offending, unpatched kernels are no longer in use. */ > +static bool > +extent_need_sync (void) > +{ > + static int need_sync = -1; > + > + if (need_sync == -1) > + { > + struct utsname name; > + need_sync = 0; /* No workaround by default. */ > + > +#ifdef __linux__ > + if (uname (&name) != -1 && strncmp (name.release, "2.6.", 4) == 0) > + { > + unsigned long val; > + if (xstrtoul (name.release + 4, NULL, 0, &val, NULL) == > LONGINT_OK) That 3rd argument is the conversion base. Leaving it as 0 lets us accept octal and hexadecimal. No big risk, obviously, but you can tighten it up by using 10 instead. > + { > + if (val < 38) > + need_sync = 1; > + } > + } > +#endif > + } > + > + return need_sync; > +}