[PATCH 2/2] Add new --sync-per-filesystem to use FS_IOC_SYNCFS on unpack
To avoid mismatched data and metadata on file systems (like XFS, ext4,
btrfs, and ubifs) that delay allocation of blocks for new files,
current dpkg calls fsync for each file it unpacks before renaming it.
Unfortunately that results in an unpleasant and seeky I/O pattern, as
each file is flushed out one at a time.
The alternative of using sync generally has acceptable performance,
except it provokes I/O on unrelated filesystems (think: networked
/home or flash /media/thumb). Unfortunately there does not seem to be
a convenient system call between the two extremes of fsync and sync.
The proposed FS_IOC_SYNCFS ioctl fills that gap: when called, it syncs
the filesystem containing the given fd. This patch teaches dpkg a
--sync-per-filesystem to use that ioctl in place of fsync for the
first file associated to a given device and to skip the remaining
fsyncs for that device.
If the ioctl fails (for example due to lack of kernel support), the
--sync-per-filesystem flag is cleared and dpkg returns to the old
behavior.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end of the series. Thanks for reading.
Testing requires an "autoreconf -fis".
configure.ac | 2 +-
man/dpkg.1 | 7 ++++
src/archives.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/main.c | 4 ++-
src/main.h | 2 +-
5 files changed, 101 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index 96c6c5e..180e554 100644
--- a/configure.ac
+++ b/configure.ac
@@ -58,7 +58,7 @@ fi
# Checks for header files.
AC_HEADER_STDC
AC_CHECK_HEADERS([stddef.h error.h locale.h libintl.h kvm.h \
- sys/cdefs.h sys/syscall.h linux/fiemap.h])
+ sys/cdefs.h sys/syscall.h linux/fiemap.h linux/fs.h])
# Checks for typedefs, structures, and compiler characteristics.
AC_C_BIGENDIAN
diff --git a/man/dpkg.1 b/man/dpkg.1
index 68146f8..c1f0ffa 100644
--- a/man/dpkg.1
+++ b/man/dpkg.1
@@ -626,6 +626,13 @@ or keep.
\fB\-\-no\-debsig\fP
Do not try to verify package signatures.
.TP
+.B \-\-sync\-per\-filesystem
+Use FS_IOC_SYNCFS ioctl if available when unpacking. This means each
+file system touched in the course of unpacking is sync'd all at once,
+which tends to result in a faster I/O pattern than syncing files one
+at a time. To disable the syncs altogether, use the
+\-\-force\-unsafe\-io option instead.
+.TP
\fB\-\-no\-triggers\fP
Do not run any triggers in this run (activations will still be recorded).
If used with \fB\-\-configure\fP \fIpackage\fP or
diff --git a/src/archives.c b/src/archives.c
index d68f9d6..d75d6a9 100644
--- a/src/archives.c
+++ b/src/archives.c
@@ -56,6 +56,14 @@
#include <selinux/selinux.h>
#endif
+#ifdef HAVE_LINUX_FS_H
+#include <linux/fs.h>
+#include <sys/ioctl.h>
+#ifndef FS_IOC_SYNCFS
+#define FS_IOC_SYNCFS _IO('f', 12)
+#endif
+#endif
+
#include "filesdb.h"
#include "main.h"
#include "archives.h"
@@ -858,11 +866,90 @@ tarobject(void *ctx, struct tar_entry *ti)
return 0;
}
+/* Singly linked list of device numbers. */
+struct synched_device {
+ struct synched_device *next;
+ dev_t device;
+};
+typedef struct synched_device *device_list;
+#define DEVICE_LIST_INIT NULL
+
+static void
+device_list_free(device_list devices)
+{
+ struct synched_device *p, *next;
+ for (p = devices; p; p = next) {
+ next = p->next;
+ free(p);
+ }
+}
+
+static void
+device_add(struct synched_device **result, dev_t devno)
+{
+ struct synched_device *node = m_malloc(sizeof(*node));
+
+ node->next = NULL;
+ node->device = devno;
+ *result = node;
+}
+
+static bool
+device_seen(device_list *devices, dev_t devno)
+{
+ struct synched_device **next = devices;
+ struct synched_device *p;
+
+ for (p = *devices; p; next = &p->next, p = *next) {
+ if (p->device == devno)
+ return true;
+ }
+ device_add(next, devno);
+ return false;
+}
+
+#ifdef FS_IOC_SYNCFS
+static int
+sync_filesystem(int fd)
+{
+ return ioctl(fd, FS_IOC_SYNCFS);
+}
+#else
+static int
+sync_filesystem(int fd)
+{
+ errno = ENOTSUP;
+ return -1;
+}
+#endif
+
+static int
+fsync_or_sync_filesystem(device_list *synched, int fd)
+{
+ struct stat stbuf;
+
+ if (!f_syncperfs)
+ return fsync(fd);
+ if (fstat(fd, &stbuf))
+ return -1;
+ if (device_seen(synched, stbuf.st_dev)) {
+ debug(dbg_eachfiledetail, "filesystem already synched");
+ return 0;
+ }
+ if (sync_filesystem(fd)) {
+ debug(dbg_eachfiledetail, "sync_filesystem failed; falling back to fsync");
+ f_syncperfs = 0;
+ return fsync(fd);
+ }
+ return 0;
+}
+
void
tar_deferred_extract(struct fileinlist *files, struct pkginfo *pkg)
{
struct fileinlist *cfile;
struct filenamenode *usenode;
+ device_list synched = DEVICE_LIST_INIT;
const char *usename;
for (cfile = files; cfile; cfile = cfile->next) {
@@ -884,7 +971,7 @@ tar_deferred_extract(struct fileinlist *files, struct pkginfo *pkg)
fd = open(fnamenewvb.buf, O_WRONLY);
if (fd < 0)
ohshite(_("unable to open '%.255s'"), fnamenewvb.buf);
- if (fsync(fd))
+ if (fsync_or_sync_filesystem(&synched, fd))
ohshite(_("unable to sync file '%.255s'"), fnamenewvb.buf);
if (close(fd))
ohshite(_("error closing/writing `%.255s'"), fnamenewvb.buf);
@@ -912,6 +999,7 @@ tar_deferred_extract(struct fileinlist *files, struct pkginfo *pkg)
debug(dbg_eachfiledetail, "deferred extract done and installed");
}
+ device_list_free(synched);
}
/**
diff --git a/src/main.c b/src/main.c
index 880ed75..eace76e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -136,6 +136,7 @@ usage(const struct cmdinfo *ci, const char *value)
" -G|--refuse-downgrade Skip packages with earlier version than installed.\n"
" -B|--auto-deconfigure Install even if it would break some other package.\n"
" --[no-]triggers Skip or force consequential trigger processing.\n"
+" --sync-per-filesystem Use sync_filesystem instead of fsync if available.\n"
" --no-debsig Do not try to verify package signatures.\n"
" --no-act|--dry-run|--simulate\n"
" Just say what we would do - don't do it.\n"
@@ -178,7 +179,7 @@ const char printforhelp[]= N_(
int f_pending=0, f_recursive=0, f_alsoselect=1, f_skipsame=0, f_noact=0;
int f_autodeconf=0, f_nodebsig=0;
-int f_triggers = 0;
+int f_triggers = 0, f_syncperfs = 0;
unsigned long f_debug=0;
int fc_downgrade=1, fc_configureany=0, fc_hold=0, fc_removereinstreq=0, fc_overwrite=0;
int fc_removeessential=0, fc_conflicts=0, fc_depends=0, fc_dependsversion=0;
@@ -539,6 +540,7 @@ static const struct cmdinfo cmdinfos[]= {
{ "no-also-select", 'N', 0, &f_alsoselect, NULL, NULL, 0 },
{ "skip-same-version", 'E', 0, &f_skipsame, NULL, NULL, 1 },
{ "auto-deconfigure", 'B', 0, &f_autodeconf, NULL, NULL, 1 },
+ { "sync-per-filesystem", 0, 0, &f_syncperfs, NULL, NULL, 1 },
{ "root", 0, 1, NULL, NULL, setroot, 0 },
{ "abort-after", 0, 1, &errabort, NULL, setinteger, 0 },
{ "admindir", 0, 1, NULL, &admindir, NULL, 0 },
diff --git a/src/main.h b/src/main.h
index a7142af..70e5526 100644
--- a/src/main.h
+++ b/src/main.h
@@ -123,7 +123,7 @@ extern const char *const statusstrings[];
extern int f_pending, f_recursive, f_alsoselect, f_skipsame, f_noact;
extern int f_autodeconf, f_nodebsig;
-extern int f_triggers;
+extern int f_triggers, f_syncperfs;
extern unsigned long f_debug;
extern int fc_downgrade, fc_configureany, fc_hold, fc_removereinstreq, fc_overwrite;
extern int fc_removeessential, fc_conflicts, fc_depends, fc_dependsversion;
--
1.7.2.3
Reply to: