From db94550cd716aec22def3e9cb873e74e980ba96a Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Mon, 7 Sep 2026 21:16:55 +0200 Subject: [PATCH] Remove physical offset sorting support and default to directory order --- AGENTS.md | 2 +- Makefile.am | 10 ++-- cmdline/elem.c | 17 +----- cmdline/elem.h | 9 +-- cmdline/scan.c | 133 ++------------------------------------------- cmdline/snapraid.c | 5 -- cmdline/state.c | 8 +-- cmdline/state.h | 3 +- 8 files changed, 18 insertions(+), 169 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1a8ecc0c..4f5c8204 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ The platform abstraction layer is separated into `os/`. Note that `os/` acts as | `snapraid_state` | `state.h` | Global program state: all disks, parity levels, filter list, options, hash kind, progress | | `snapraid_option` | `state.h` | Runtime options (force flags, sort order, IO advise mode, bandwidth limit, etc.) | | `snapraid_disk` | `elem.h` | One data disk: name, mount dir, device id, UUID, file/link/dir hash tables and lists, parity extent tree | -| `snapraid_file` | `elem.h` | A tracked file: sub-path, size, mtime, inode, physical offset, block vector | +| `snapraid_file` | `elem.h` | A tracked file: sub-path, size, mtime, inode, block vector | | `snapraid_block` | `elem.h` | One parity block: state (`BLK/CHG/REP/DELETED/EMPTY`) + hash | | `snapraid_extent` | `elem.h` | Contiguous run of file blocks mapped to contiguous parity positions | | `snapraid_filter` | `elem.h` | One include/exclude rule: pattern, root scope, `is_disk`, `is_abs`, `is_dir`, direction | diff --git a/Makefile.am b/Makefile.am index 1a2a54ee..bca4b92e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -293,8 +293,8 @@ CHECKFLAGS_COMMON = --test-skip-device --test-skip-self --test-force-progress -- # the inode, physical location, and dir order assigned by the OS. CHECKFLAGS_ALPHA = $(CHECKFLAGS_COMMON) --test-force-order-alpha -# Physical offset options -CHECKFLAGS_PHYSICAL = $(CHECKFLAGS_COMMON) --test-force-order-physical -q -q -q +# Directory order options +CHECKFLAGS_DIR = $(CHECKFLAGS_COMMON) --test-force-order-dir -q -q -q # Verbose options CHECKFLAGS_VERBOSE = $(CHECKFLAGS_ALPHA) -v --gui @@ -601,9 +601,9 @@ endif # HAVE_POSIX echo DUP > bench/disk1/DUP2 echo -n > bench/disk1/ZERO $(MSG) Some commands with a not empty array -# Run a sync using physical offset - $(FAILENV) $(SNAPRAID) $(CHECKFLAGS_PHYSICAL) -c $(CONF) --test-expect-need-sync diff > output.log - $(TESTENV) $(SNAPRAID) $(CHECKFLAGS_PHYSICAL) -c $(CONF) sync +# Run a sync using directory order + $(FAILENV) $(SNAPRAID) $(CHECKFLAGS_DIR) -c $(CONF) --test-expect-need-sync diff > output.log + $(TESTENV) $(SNAPRAID) $(CHECKFLAGS_DIR) -c $(CONF) sync # Now reset the array, as we normally test with alpha order and murmur3 rm bench/content bench/?-content # Now rebuild the array with alpha order and murmur3 and do some commands diff --git a/cmdline/elem.c b/cmdline/elem.c index a1bfc050..b5cb3be5 100644 --- a/cmdline/elem.c +++ b/cmdline/elem.c @@ -469,7 +469,7 @@ int filter_snapshot(int enable, const char* sub, const char* name) return 0; } -struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode, uint64_t physical) +struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode) { struct snapraid_file* file; block_off_t i; @@ -495,7 +495,6 @@ struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_ file->mtime_sec = mtime_sec; file->mtime_nsec = mtime_nsec; file->inode = inode; - file->physical = physical; file->flag = 0; file->blockvec = nalloc_nofail((size_t)file->blockmax, block_sizeof()); @@ -529,7 +528,6 @@ struct snapraid_file* file_dup(struct snapraid_file* copy) file->mtime_sec = copy->mtime_sec; file->mtime_nsec = copy->mtime_nsec; file->inode = copy->inode; - file->physical = copy->physical; file->flag = copy->flag; file->blockvec = nalloc_nofail((size_t)file->blockmax, block_sizeof()); @@ -670,18 +668,6 @@ int file_path_compare(const void* void_a, const void* void_b) return strcmp(file_a->sub, file_b->sub); } -int file_physical_compare(const void* void_a, const void* void_b) -{ - const struct snapraid_file* file_a = void_a; - const struct snapraid_file* file_b = void_b; - - if (file_a->physical < file_b->physical) - return -1; - if (file_a->physical > file_b->physical) - return 1; - return 0; -} - int file_path_compare_to_arg(const void* void_arg, const void* void_data) { const char* arg = void_arg; @@ -981,7 +967,6 @@ struct snapraid_disk* disk_alloc(const char* name, const char* dir, uint64_t dev disk->first_free_block = 0; disk->has_volatile_inodes = 0; disk->has_volatile_hardlinks = 0; - disk->has_unreliable_physical = 0; disk->has_different_uuid = 0; disk->has_unsupported_uuid = *uuid == 0; /* empty UUID means unsupported */ disk->had_empty_uuid = 0; diff --git a/cmdline/elem.h b/cmdline/elem.h index bf65488a..6e71169e 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -329,7 +329,6 @@ struct snapraid_block { struct snapraid_file { int64_t mtime_sec; /**< Modification time. */ uint64_t inode; /**< Inode. */ - uint64_t physical; /**< Physical offset of the file. */ data_off_t size; /**< Size of the file. */ struct snapraid_block* blockvec; /**< All the blocks of the file. */ block_off_t blockmax; /**< Number of blocks. */ @@ -468,7 +467,6 @@ struct snapraid_disk { int has_volatile_inodes; /**< If the underline file-system has not persistent inodes. */ int has_volatile_hardlinks; /**< If the underline file-system has not synchronized metadata for hardlink (NTFS). */ - int has_unreliable_physical; /**< If the physical offset of files has duplicates. */ int has_different_uuid; /**< If the disk has a different UUID, meaning that it is not the same file-system. */ int has_unsupported_uuid; /**< If the disk doesn't report UUID, meaning it's not supported. */ int had_empty_uuid; /**< If the disk had an empty UUID, meaning that it's a new disk. */ @@ -887,7 +885,7 @@ static inline void file_flag_clear(struct snapraid_file* file, unsigned mask) /** * Allocate a file. */ -struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode, uint64_t physical); +struct snapraid_file* file_alloc(unsigned block_size, const char* sub, data_off_t size, uint64_t mtime_sec, int mtime_nsec, uint64_t inode); /** * Duplicate a file. @@ -952,11 +950,6 @@ int file_inode_compare(const void* void_a, const void* void_b); */ int file_path_compare(const void* void_a, const void* void_b); -/** - * Compare files by physical address. - */ -int file_physical_compare(const void* void_a, const void* void_b); - /** * Compute the hash of a file inode. */ diff --git a/cmdline/scan.c b/cmdline/scan.c index a1502090..df3cb0d8 100644 --- a/cmdline/scan.c +++ b/cmdline/scan.c @@ -42,7 +42,6 @@ struct snapraid_discovery { data_off_t size; /**< Size discovered on disk. */ int64_t mtime_sec; /**< mtime sec discovered on disk. */ uint64_t inode; /**< Inode discovered on disk. */ - uint64_t physical; /**< Physical offset. */ tommy_node nodelist; /**< Node for scan->file_discovery_list. */ tommy_node nodeset; /**< Node for scan->file_discovery_inodeset. */ int mtime_nsec; /**< mtime nsec discovered on disk. */ @@ -92,7 +91,6 @@ static struct snapraid_discovery* discovery_alloc(const char* sub, struct stat* disc->mtime_sec = st->st_mtime; disc->mtime_nsec = STAT_NSEC(st); disc->inode = st->st_ino; - disc->physical = FILEPHY_UNREAD_OFFSET; disc->flag = 0; memcpy(disc->sub, sub, sub_len + 1); @@ -648,7 +646,7 @@ static void scan_file_remove(struct snapraid_scan* scan, struct snapraid_file* f * This could happen after a failed sync, when some other files are deleted, * and then new ones can be moved backward to fill the hole created. */ -static void scan_file_keep(struct snapraid_scan* scan, struct snapraid_file* file, struct snapraid_discovery* disc) +static void scan_file_keep(struct snapraid_scan* scan, struct snapraid_file* file) { struct snapraid_disk* disk = scan->disk; @@ -658,10 +656,6 @@ static void scan_file_keep(struct snapraid_scan* scan, struct snapraid_file* fil struct snapraid_file* copy = file_dup(file); file_flag_set(copy, FILE_IS_REALLOC_NEW); - /* update physical offset if provided by discovery */ - if (disc->physical != FILEPHY_UNREAD_OFFSET) - copy->physical = disc->physical; - /* insert in the delayed allocation list */ scan_file_delayed_allocate(scan, copy); @@ -797,7 +791,7 @@ static void scan_file_apply(void* void_scan, void* void_disc) } /* mark the file as kept */ - scan_file_keep(scan, file, disc); + scan_file_keep(scan, file); /* nothing more to do */ return; @@ -941,7 +935,7 @@ static void scan_file_apply(void* void_scan, void* void_disc) } /* mark the file as kept */ - scan_file_keep(scan, file, disc); + scan_file_keep(scan, file); /* nothing more to do */ return; @@ -1001,7 +995,7 @@ static void scan_file_apply(void* void_scan, void* void_disc) #endif /* insert it */ - file = file_alloc(state->block_size, sub, disc->size, disc->mtime_sec, disc->mtime_nsec, disc->inode, disc->physical); + file = file_alloc(state->block_size, sub, disc->size, disc->mtime_sec, disc->mtime_nsec, disc->inode); /* mark it as present and physically discovered during Phase 1 */ file_flag_set(file, FILE_IS_PRESENT); @@ -1784,66 +1778,9 @@ static int scan_dir(struct snapraid_scan* scan, int level, int is_diff, const ch return scan_sub(scan, level, is_diff, path_next, sub_next, tmp); } -/** - * Resolve physical offset for a single discovered regular file entry. - * - * Hardlink aliases are skipped because they are never allocated in parity and do not - * participate in physical ordering. - */ -static void scan_discovery_physical_entry(void* void_scan, void* void_disc) -{ - struct snapraid_scan* scan = void_scan; - struct snapraid_state* state = scan->state; - struct snapraid_disk* disk = scan->disk; - struct snapraid_discovery* disc = void_disc; - struct snapraid_file* existing; - char path_next[PATH_MAX]; - - /* skip hardlink aliases; only canonical representatives or standalone files need physical offsets */ - if (disc->inode != INODE_INVALID && (disc->flag & DISCOVERY_IS_CANONICAL) == 0) - return; - - /* search existing file by path, or by inode if moved or promoted */ - existing = tommy_hashdyn_search(&disk->pathset, file_path_compare_to_arg, disc->sub, file_path_hash(disc->sub)); - if (!existing) { - if (!disk->has_volatile_inodes && !disk->has_different_uuid && !disk->has_unsupported_uuid && disc->inode != INODE_INVALID) - existing = tommy_hashdyn_search(&disk->inodeset, file_inode_compare_to_arg, &disc->inode, file_inode_hash(disc->inode)); - } - - /* reuse physical offset if the file exists unchanged and is not being reallocated */ - if (existing - && existing->size == disc->size - && existing->mtime_sec == disc->mtime_sec - && existing->mtime_nsec == disc->mtime_nsec - && !file_is_full_reallocatable_and_stable(state, disk, existing) - ) { - disc->physical = existing->physical; - return; - } - - pathprint(path_next, sizeof(path_next), "%s%s", disk->dir, disc->sub); - - if (filephy(path_next, disc->size, &disc->physical) != 0) { - /* LCOV_EXCL_START */ - log_tag("%s:%u:%s:%s: File physycal offset error. %s.\n", es(errno), 0, disk->name, esc_tag(disc->sub), strerror(errno)); - log_fatal(errno, "Error in getting the physical offset of file '%s'. %s.\n", path_next, strerror(errno)); - exit(EXIT_FAILURE); - /* LCOV_EXCL_STOP */ - } -} - -/** - * Resolve physical offsets for canonical files and standalone regular files in Phase 1B. - */ -static void scan_discovery_physical(struct snapraid_scan* scan) -{ - tommy_list_foreach_arg(&scan->file_discovery_list, scan_discovery_physical_entry, scan); -} - static void* scan_disk(void* arg) { struct snapraid_scan* scan = arg; - struct snapraid_state* state = scan->state; struct snapraid_disk* disk = scan->disk; int ret; int has_persistent_inodes; @@ -1910,9 +1847,6 @@ static void* scan_disk(void* arg) scan_dir(scan, 0, scan->is_diff, disk->dir, ""); - if (state->opt.force_order == SORT_PHYSICAL) - scan_discovery_physical(scan); - if (!scan->is_diff) msg_progress("Scanned %s in %" PRIu64 " seconds\n", disk->name, (os_tick_ms() - start) / 1000); @@ -2033,9 +1967,6 @@ static int state_diffscan(struct snapraid_state* state, int is_diff) struct snapraid_scan* scan = i->data; struct snapraid_disk* disk = scan->disk; tommy_node* node; - unsigned phy_dup; - uint64_t phy_last; - struct snapraid_file* phy_file_last; /* * Phase 3: Removals (deleted files and old versions of modified files) @@ -2144,13 +2075,10 @@ static int state_diffscan(struct snapraid_state* state, int is_diff) * state_fscheck() validates the resulting structures after all disks finish. * * Sort the files before inserting them - * we use a stable sort to ensure that if the reported physical offset/inode + * we use a stable sort to ensure that if the reported inode * are always 0, we keep at least the directory order */ switch (state->opt.force_order) { - case SORT_PHYSICAL : - tommy_list_sort(&scan->file_insert_list, file_physical_compare); - break; case SORT_INODE : tommy_list_sort(&scan->file_insert_list, file_inode_compare); break; @@ -2165,34 +2093,11 @@ static int state_diffscan(struct snapraid_state* state, int is_diff) /* * Insert all the new files, we insert them only after the deletion * to reuse the just freed space - * also check if the physical offset reported are fakes or not */ node = scan->file_insert_list; - phy_dup = 0; - phy_last = FILEPHY_UNREAD_OFFSET; - phy_file_last = 0; while (node) { struct snapraid_file* file = node->data; - /* if the file is not empty, count duplicate physical offsets */ - if (state->opt.force_order == SORT_PHYSICAL && file->size != 0) { - if (phy_file_last != 0 && file->physical == phy_last - /* files without offset are expected to have duplicates */ - && phy_last != FILEPHY_WITHOUT_OFFSET - ) { - /* - * If verbose, print the list of duplicates real offsets - * other cases are for offsets not supported, so we don't need to report them file by file - */ - if (phy_last >= FILEPHY_REAL_OFFSET) { - log_info(ESOFT, "WARNING! Files '%s%s' and '%s%s' share the same physical offset %" PRId64 ".\n", disk->mount_point, phy_file_last->sub, disk->mount_point, file->sub, phy_last); - } - ++phy_dup; - } - phy_file_last = file; - phy_last = file->physical; - } - /* next node */ node = node->next; @@ -2213,14 +2118,6 @@ static int state_diffscan(struct snapraid_state* state, int is_diff) scan_file_allocate(scan, file); } - /* - * Mark the disk without reliable physical offset if it has duplicates - * here it should never happen because we already sorted out hardlinks - */ - if (state->opt.force_order == SORT_PHYSICAL && phy_dup > 0) { - disk->has_unreliable_physical = 1; - } - /* insert all the new links */ node = scan->link_insert_list; while (node) { @@ -2304,26 +2201,6 @@ static int state_diffscan(struct snapraid_state* state, int is_diff) } } - /* check for disks without the physical offset support */ - if (state->opt.force_order == SORT_PHYSICAL) { - done = 0; - for (i = state->disklist; i != 0; i = i->next) { - struct snapraid_disk* disk = i->data; - - if (disk->has_unreliable_physical) { - if (!done) { - done = 1; - log_info(ESOFT, "WARNING! Physical offsets not supported for disk '%s'", disk->name); - } else { - log_info(ESOFT, ", '%s'", disk->name); - } - } - } - if (done) { - log_info(ESOFT, ". The order of files won't be optimal.\n"); - } - } - /* check for disks without persistent inodes */ done = 0; for (i = state->disklist; i != 0; i = i->next) { diff --git a/cmdline/snapraid.c b/cmdline/snapraid.c index 281047f4..2868ac0e 100644 --- a/cmdline/snapraid.c +++ b/cmdline/snapraid.c @@ -217,7 +217,6 @@ void log_close(const char* file) #define OPT_TEST_FORCE_MURMUR3 264 #define OPT_TEST_FORCE_SPOOKY2 265 #define OPT_TEST_SKIP_LOCK 266 -#define OPT_TEST_FORCE_ORDER_PHYSICAL 267 #define OPT_TEST_FORCE_ORDER_INODE 268 #define OPT_TEST_FORCE_ORDER_ALPHA 269 #define OPT_TEST_FORCE_ORDER_DIR 270 @@ -347,7 +346,6 @@ static struct option long_options[] = { { "test-skip-lock", 0, 0, OPT_TEST_SKIP_LOCK }, /* Force a sort order for files */ - { "test-force-order-physical", 0, 0, OPT_TEST_FORCE_ORDER_PHYSICAL }, { "test-force-order-inode", 0, 0, OPT_TEST_FORCE_ORDER_INODE }, { "test-force-order-alpha", 0, 0, OPT_TEST_FORCE_ORDER_ALPHA }, { "test-force-order-dir", 0, 0, OPT_TEST_FORCE_ORDER_DIR }, @@ -907,9 +905,6 @@ int snapraid_main(int argc, char* argv[]) case OPT_TEST_SKIP_LOCK : opt.skip_lock = 1; break; - case OPT_TEST_FORCE_ORDER_PHYSICAL : - opt.force_order = SORT_PHYSICAL; - break; case OPT_TEST_FORCE_ORDER_INODE : opt.force_order = SORT_INODE; break; diff --git a/cmdline/state.c b/cmdline/state.c index 9f02beda..0fd1c401 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -645,9 +645,9 @@ void state_config(struct snapraid_state* state, const char* path, const char* co /* copy the options */ state->opt = *opt; - /* if unset, sort by physical order */ + /* if unset, sort by directory order */ if (!state->opt.force_order) - state->opt.force_order = SORT_PHYSICAL; + state->opt.force_order = SORT_DIR; /* adjust file mode */ if (state->opt.file_mode != ADVISE_DEFAULT) { @@ -2417,7 +2417,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S } /* allocate the file */ - file = file_alloc(state->block_size, sub, v_size, v_mtime_sec, v_mtime_nsec, v_inode, FILEPHY_UNREAD_OFFSET); + file = file_alloc(state->block_size, sub, v_size, v_mtime_sec, v_mtime_nsec, v_inode); /* insert the file in the file containers */ if (file->inode != INODE_INVALID) @@ -2745,7 +2745,7 @@ static void state_read_content(struct snapraid_state* state, const char* path, S } /* allocate a fake deleted file */ - deleted = file_alloc(state->block_size, "", v_count * (data_off_t)state->block_size, 0, 0, 0, 0); + deleted = file_alloc(state->block_size, "", v_count * (data_off_t)state->block_size, 0, 0, 0); /* mark the file as deleted */ file_flag_set(deleted, FILE_IS_DELETED); diff --git a/cmdline/state.h b/cmdline/state.h index d4d0c060..30fdf93f 100644 --- a/cmdline/state.h +++ b/cmdline/state.h @@ -42,10 +42,9 @@ const char* lev_config_name(unsigned level); #define GIBI (1024 * 1024 * 1024) #define TEBI (1024 * 1024 * 1024 * (uint64_t)1024) -#define SORT_PHYSICAL 1 /**< Sort by physical order. */ +#define SORT_DIR 1 /**< Sort by directory order. */ #define SORT_INODE 2 /**< Sort by inode. */ #define SORT_ALPHA 3 /**< Sort by alphabetic order. */ -#define SORT_DIR 4 /**< Sort by directory order. */ /** * Options set only at startup.