Parcourir la source

dpkg: Defer the fsync and rename for normal files in tar extraction

This way it's done in one pass afterwards, to avoid massive I/O
degradation due to the serialization from each write + fsync. This
restores extraction times to numbers closer to the ones before the
fsync patch introduced in 1.15.6.
Guillem Jover il y a 16 ans
Parent
commit
9cd41fdda1
5 fichiers modifiés avec 83 ajouts et 12 suppressions
  1. 4 0
      debian/changelog
  2. 74 12
      src/archives.c
  3. 1 0
      src/archives.h
  4. 2 0
      src/filesdb.h
  5. 2 0
      src/processarc.c

+ 4 - 0
debian/changelog

@@ -12,6 +12,10 @@ dpkg (1.15.6.1) UNRELEASED; urgency=low
     the code to use STL's min() and max() instead of preprocessor macros, to
     the code to use STL's min() and max() instead of preprocessor macros, to
     avoid multiple evaluation of arguments. Regression introduced in 1.15.6.
     avoid multiple evaluation of arguments. Regression introduced in 1.15.6.
     Based on a patch by Robert Luberda <robert@debian.org>. Closes: #574816
     Based on a patch by Robert Luberda <robert@debian.org>. Closes: #574816
+  * Defer the fsync and rename for normal files in tar extraction so that
+    it's done in one pass afterwards, to avoid massive I/O degradation due to
+    the serialization from each write + fsync. This restores extraction times
+    to numbers closer to the ones before the fsync patch introduced in 1.15.6.
 
 
   [ Raphaël Hertzog ]
   [ Raphaël Hertzog ]
   * Accept source packages without "Format" field for compatibility with very
   * Accept source packages without "Format" field for compatibility with very

+ 74 - 12
src/archives.c

@@ -661,8 +661,10 @@ int tarobject(struct TarInfo *ti) {
     am=(nifd->namenode->statoverride ? nifd->namenode->statoverride->mode : ti->Mode) & ~S_IFMT;
     am=(nifd->namenode->statoverride ? nifd->namenode->statoverride->mode : ti->Mode) & ~S_IFMT;
     if (fchmod(fd,am))
     if (fchmod(fd,am))
       ohshite(_("error setting permissions of `%.255s'"),ti->Name);
       ohshite(_("error setting permissions of `%.255s'"),ti->Name);
-    if (fsync(fd))
-      ohshite(_("unable to sync file '%.255s'"), ti->Name);
+
+    /* Postpone the fsync, to try to avoid massive I/O degradation. */
+    nifd->namenode->flags |= fnnf_deferred_fsync;
+
     pop_cleanup(ehflag_normaltidy); /* fd= open(fnamenewvb.buf) */
     pop_cleanup(ehflag_normaltidy); /* fd= open(fnamenewvb.buf) */
     if (close(fd))
     if (close(fd))
       ohshite(_("error closing/writing `%.255s'"),ti->Name);
       ohshite(_("error closing/writing `%.255s'"),ti->Name);
@@ -777,22 +779,82 @@ int tarobject(struct TarInfo *ti) {
    * in dpkg-new.
    * in dpkg-new.
    */
    */
 
 
-  if (rename(fnamenewvb.buf,fnamevb.buf))
-    ohshite(_("unable to install new version of `%.255s'"),ti->Name);
+  if (ti->Type == NormalFile0 || ti->Type == NormalFile0) {
+    nifd->namenode->flags |= fnnf_deferred_rename;
 
 
-  /* CLEANUP: now the new file is in the destination file, and the
-   * old file is in dpkg-tmp to be cleaned up later.  We now need
-   * to take a different attitude to cleanup, because we need to
-   * remove the new file.
-   */
+    debug(dbg_eachfiledetail, "tarobject done and installation deferred");
+  } else {
+    if (rename(fnamenewvb.buf, fnamevb.buf))
+      ohshite(_("unable to install new version of `%.255s'"), ti->Name);
 
 
-  nifd->namenode->flags |= fnnf_placed_on_disk;
-  nifd->namenode->flags |= fnnf_elide_other_lists;
+    /* CLEANUP: now the new file is in the destination file, and the
+     * old file is in dpkg-tmp to be cleaned up later.  We now need
+     * to take a different attitude to cleanup, because we need to
+     * remove the new file. */
+
+    nifd->namenode->flags |= fnnf_placed_on_disk;
+    nifd->namenode->flags |= fnnf_elide_other_lists;
+
+    debug(dbg_eachfiledetail, "tarobject done and installed");
+  }
 
 
-  debug(dbg_eachfiledetail,"tarobject done and installed");
   return 0;
   return 0;
 }
 }
 
 
+void
+tar_deferred_extract(struct pkginfo *pkg)
+{
+  struct fileinlist *cfile;
+  struct filenamenode *usenode;
+  const char *usename;
+
+  for (cfile = pkg->clientdata->files; cfile; cfile = cfile->next) {
+    debug(dbg_eachfile, "deferred extract of '%.255s'", cfile->namenode->name);
+
+    if (!(cfile->namenode->flags & fnnf_deferred_rename))
+      continue;
+
+    debug(dbg_eachfiledetail, "deferred extract needs rename");
+
+    usenode = namenodetouse(cfile->namenode, pkg);
+    usename = usenode->name + 1; /* Skip the leading '/'. */
+
+    setupfnamevbs(usename);
+
+    if (cfile->namenode->flags & fnnf_deferred_fsync) {
+      int fd;
+
+      debug(dbg_eachfiledetail, "deferred extract needs fsync");
+
+      fd = open(fnamenewvb.buf, O_WRONLY);
+      if (fd < 0)
+        ohshite(_("unable to open '%.255s'"), fnamenewvb.buf);
+      if (fsync(fd))
+        ohshite(_("unable to sync file '%.255s'"), fnamenewvb.buf);
+      if (close(fd))
+        ohshite(_("error closing/writing `%.255s'"), fnamenewvb.buf);
+
+      cfile->namenode->flags &= ~fnnf_deferred_fsync;
+    }
+
+    if (rename(fnamenewvb.buf, fnamevb.buf))
+      ohshite(_("unable to install new version of `%.255s'"),
+              cfile->namenode->name);
+
+    cfile->namenode->flags &= ~fnnf_deferred_rename;
+
+    /* CLEANUP: now the new file is in the destination file, and the
+     * old file is in dpkg-tmp to be cleaned up later.  We now need
+     * to take a different attitude to cleanup, because we need to
+     * remove the new file. */
+
+    cfile->namenode->flags |= fnnf_placed_on_disk;
+    cfile->namenode->flags |= fnnf_elide_other_lists;
+
+    debug(dbg_eachfiledetail, "deferred extract done and installed");
+  }
+}
+
 static int
 static int
 try_deconfigure_can(bool (*force_p)(struct deppossi *), struct pkginfo *pkg,
 try_deconfigure_can(bool (*force_p)(struct deppossi *), struct pkginfo *pkg,
                     struct deppossi *pdep, const char *action,
                     struct deppossi *pdep, const char *action,

+ 1 - 0
src/archives.h

@@ -66,6 +66,7 @@ int unlinkorrmdir(const char *filename);
 
 
 int tarobject(struct TarInfo *ti);
 int tarobject(struct TarInfo *ti);
 int tarfileread(void *ud, char *buf, int len);
 int tarfileread(void *ud, char *buf, int len);
+void tar_deferred_extract(struct pkginfo *pkg);
 
 
 bool filesavespackage(struct fileinlist *, struct pkginfo *,
 bool filesavespackage(struct fileinlist *, struct pkginfo *,
                       struct pkginfo *pkgbeinginstalled);
                       struct pkginfo *pkgbeinginstalled);

+ 2 - 0
src/filesdb.h

@@ -69,6 +69,8 @@ struct filenamenode {
     fnnf_elide_other_lists=   000010, /* must remove from other packages' lists */
     fnnf_elide_other_lists=   000010, /* must remove from other packages' lists */
     fnnf_no_atomic_overwrite= 000020, /* >=1 instance is a dir, cannot rename over */
     fnnf_no_atomic_overwrite= 000020, /* >=1 instance is a dir, cannot rename over */
     fnnf_placed_on_disk=      000040, /* new file has been placed on the disk */
     fnnf_placed_on_disk=      000040, /* new file has been placed on the disk */
+    fnnf_deferred_fsync =     000200,
+    fnnf_deferred_rename =    000400,
   } flags; /* Set to zero when a new node is created. */
   } flags; /* Set to zero when a new node is created. */
   const char *oldhash; /* valid iff this namenode is in the newconffiles list */
   const char *oldhash; /* valid iff this namenode is in the newconffiles list */
   struct stat *filestat;
   struct stat *filestat;

+ 2 - 0
src/processarc.c

@@ -639,6 +639,8 @@ void process_archive(const char *filename) {
   p1[0] = -1;
   p1[0] = -1;
   subproc_wait_check(c1, BACKEND " --fsys-tarfile", PROCPIPE);
   subproc_wait_check(c1, BACKEND " --fsys-tarfile", PROCPIPE);
 
 
+  tar_deferred_extract(pkg);
+
   if (oldversionstatus == stat_halfinstalled || oldversionstatus == stat_unpacked) {
   if (oldversionstatus == stat_halfinstalled || oldversionstatus == stat_unpacked) {
     /* Packages that were in `installed' and `postinstfailed' have been reduced
     /* Packages that were in `installed' and `postinstfailed' have been reduced
      * to `unpacked' by now, by the running of the prerm script.
      * to `unpacked' by now, by the running of the prerm script.