Bladeren bron

Removed file outlining on ENOSPC in lfs_file_sync

This was initially added as protection against the case where a file
grew to no longer fit in a metadata-pair. While in most cases this
should be caught by the math in lfs_file_write, it doesn't handle a
problem that can happen if the files metadata is large enough that even
small inline files can't fit. This can happen if you combine a small
block size with large file names and many custom attributes.

But trying to outline on ENOSPC creates creates a lot of problems.

If we are actually low on space, this is one of the worst things we can
do. Inline files take up less space than CTZ skip-lists, but inline
files are rendered useless if we outline inline files as soon as we run
low on space.

On top of this, the outlining logic tries multiple mdir commits if it
gets ENOSPC, which can hide errors if ENOSPC is returned for other
reasons.

In a perfect world, we would be using a different error code for
no-room-in-metadata-pair, and no-blocks-on-disk.

For now I've removed the outlining logic and we will need to figure out
how to handle this situation more robustly.
Christopher Haster 5 jaren geleden
bovenliggende
commit
f9c2fd93f2
1 gewijzigde bestanden met toevoegingen van 23 en 24 verwijderingen
  1. 23 24
      lfs.c

+ 23 - 24
lfs.c

@@ -1631,33 +1631,17 @@ static int lfs_dir_compact(lfs_t *lfs,
 
             // TODO huh?
             LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0);
-            // update gstate
-            lfs->gdelta = (lfs_gstate_t){0};
-            if (!relocated) {
-                lfs->gdisk = lfs->gstate;
-            }
-
-            // TODO here??
-            if (relocated) {
-                // update references if we relocated
-                LFS_DEBUG("Relocating %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32,
-                        oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
-                err = lfs_fs_relocate(lfs, oldpair, dir->pair);
-                if (err) {
-                    // TODO make better
-                    dir->pair[1] = oldpair[1]; //
-                    return err;
-                }
-                LFS_DEBUG("Relocated %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32,
-                        oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
-            }
-
             // successful compaction, swap dir pair to indicate most recent
             lfs_pair_swap(dir->pair);
             dir->rev = nrev;
             dir->count = end - begin;
             dir->off = commit.off;
             dir->etag = commit.ptag;
+            // update gstate
+            lfs->gdelta = (lfs_gstate_t){0};
+            if (!relocated) {
+                lfs->gdisk = lfs->gstate;
+            }
         }
         break;
 
@@ -1685,6 +1669,21 @@ relocate:
         continue;
     }
 
+    // TODO here??
+    if (relocated) {
+        // update references if we relocated
+        LFS_DEBUG("Relocating %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32,
+                oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
+        int err = lfs_fs_relocate(lfs, oldpair, dir->pair);
+        if (err) {
+            // TODO make better
+            //dir->pair[1] = oldpair[1]; //
+            return err;
+        }
+        LFS_DEBUG("Relocated %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32,
+                oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
+    }
+
     return 0;
 }
 
@@ -2739,9 +2738,9 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) {
                     {LFS_MKTAG(LFS_FROM_USERATTRS, file->id,
                         file->cfg->attr_count), file->cfg->attrs}));
             if (err) {
-                if (err == LFS_ERR_NOSPC && (file->flags & LFS_F_INLINE)) {
-                    goto relocate;
-                }
+//                if (err == LFS_ERR_NOSPC && (file->flags & LFS_F_INLINE)) {
+//                    goto relocate;
+//                }
                 file->flags |= LFS_F_ERRED;
                 LFS_TRACE("lfs_file_sync -> %d", err);
                 return err;