From patchwork Thu Dec 9 00:57:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Neeraj Singh (WINDOWS-SFS)" X-Patchwork-Id: 12665633 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB265C433F5 for ; Thu, 9 Dec 2021 00:57:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241715AbhLIBAw (ORCPT ); Wed, 8 Dec 2021 20:00:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45294 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241701AbhLIBAv (ORCPT ); Wed, 8 Dec 2021 20:00:51 -0500 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C079BC061746 for ; Wed, 8 Dec 2021 16:57:18 -0800 (PST) Received: by mail-wm1-x332.google.com with SMTP id d72-20020a1c1d4b000000b00331140f3dc8so3001602wmd.1 for ; Wed, 08 Dec 2021 16:57:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=1ldAv98qN2O2/MysdiN04Z8IVqnd40Vv1ASnm1gfbMs=; b=UN9MC+BRAec8zYK1bK0jJ3vkv4F9Z9ZwCvRlrOSX+2Vqq94apHDqdKjhi/gSFfx2hx 6YXvNqEi5FuBIQ3Sb8AG46SqiV9MuvI17HURRvMeWxB6IiYpK/rxuYrnJw8nyvowFeGQ ZuZSCJmp210iSdOAdsCNTTC/eS5e6LLhCI/cb07T5avW8q7jX6BTSh5fuZjDOLqjtqI8 ArlB3d/CnP8xxFT3C3QjuHaJ/bgnBGNAaEDA38zHStr+dBANdaNXH1VomhsxN6uif8dV evlZOgcKjMDsk6TXZDxMmb2gpDVAWpO/vwr2q21uKQJqKNM5w8VxcZJcVMqJxgNiMK4T 2Q/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=1ldAv98qN2O2/MysdiN04Z8IVqnd40Vv1ASnm1gfbMs=; b=Nyyz8ZKGKT+bOD91T7oTAQdsbBR9Osy/yRgpMZm/WgtE1HpjytD3t+BEOw6mJVOnky UF0q8CqtQMhFMbAzDP3kOexEC8o5xqfkYhKkV3Wa5HlZ2eQ6M8L4tKKHbRLjidlQISeM I1pBu8937KYXr/c8PI9qjBxgD7e9kszWKTotvgmojTj/Bjh+TbWy/1NKR8MG0w01JEOu 0I+79Uw7C7jLQazIN58HGVMzUVgNNY+riBmAlt/UanWT4oLUgges6hzChISnx4JnsJOs H7g/D2BkbxO6x4jSV6lZCSym2aFm3c8Eqb/OmrqN3lLO2NbWwQZjHPZEt1XFcHrsXE4I Ey1w== X-Gm-Message-State: AOAM531j8WFyvHcSSU0j0aVjirVNvOElJknXBhxIXcDBLYwoqU9TDuyu 2POJGiU770CUtGJQcQVyY7dvA2cOgUA= X-Google-Smtp-Source: ABdhPJyHIuOSnKg5A6rwcZ9U21wAz0Z7x9+ls8jDiQb2JMH9d3rT/O03azyhQsrHxdsVV38RKI0row== X-Received: by 2002:a7b:c848:: with SMTP id c8mr2837098wml.105.1639011436861; Wed, 08 Dec 2021 16:57:16 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l5sm5746470wrs.59.2021.12.08.16.57.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Dec 2021 16:57:16 -0800 (PST) Message-Id: <15edfe5150961e8ec34f880a8edc44c7754af444.1639011433.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 09 Dec 2021 00:57:10 +0000 Subject: [PATCH v3 1/4] core.fsyncmethod: add writeout-only mode Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: rsbecker@nexbridge.com, bagasdotme@gmail.com, newren@gmail.com, avarab@gmail.com, nksingh85@gmail.com, ps@pks.im, "Neeraj K. Singh" , Neeraj Singh Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Neeraj Singh From: Neeraj Singh This commit introduces the `core.fsyncMethod` configuration knob, which can currently be set to `fsync` or `writeout-only`. The new writeout-only mode attempts to tell the operating system to flush its in-memory page cache to the storage hardware without issuing a CACHE_FLUSH command to the storage controller. Writeout-only fsync is significantly faster than a vanilla fsync on common hardware, since data is written to a disk-side cache rather than all the way to a durable medium. Later changes in this patch series will take advantage of this primitive to implement batching of hardware flushes. When git_fsync is called with FSYNC_WRITEOUT_ONLY, it may fail and the caller is expected to do an ordinary fsync as needed. On Apple platforms, the fsync system call does not issue a CACHE_FLUSH directive to the storage controller. This change updates fsync to do fcntl(F_FULLFSYNC) to make fsync actually durable. We maintain parity with existing behavior on Apple platforms by setting the default value of the new core.fsyncMethod option. Signed-off-by: Neeraj Singh --- Documentation/config/core.txt | 9 ++++ Makefile | 6 +++ cache.h | 7 ++++ compat/mingw.h | 3 ++ compat/win32/flush.c | 28 +++++++++++++ config.c | 12 ++++++ config.mak.uname | 3 ++ configure.ac | 8 ++++ contrib/buildsystems/CMakeLists.txt | 3 +- environment.c | 2 +- git-compat-util.h | 24 +++++++++++ wrapper.c | 64 +++++++++++++++++++++++++++++ write-or-die.c | 10 +++-- 13 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 compat/win32/flush.c diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index c04f62a54a1..dbb134f7136 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -547,6 +547,15 @@ core.whitespace:: is relevant for `indent-with-non-tab` and when Git fixes `tab-in-indent` errors. The default tab width is 8. Allowed values are 1 to 63. +core.fsyncMethod:: + A value indicating the strategy Git will use to harden repository data + using fsync and related primitives. ++ +* `fsync` uses the fsync() system call or platform equivalents. +* `writeout-only` issues pagecache writeback requests, but depending on the + filesystem and storage hardware, data added to the repository may not be + durable in the event of a system crash. This is the default mode on macOS. + core.fsyncObjectFiles:: This boolean will enable 'fsync()' when writing object files. + diff --git a/Makefile b/Makefile index d56c0e4aadc..cba024615c9 100644 --- a/Makefile +++ b/Makefile @@ -403,6 +403,8 @@ all:: # # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC. # +# Define HAVE_SYNC_FILE_RANGE if your platform has sync_file_range. +# # Define NEEDS_LIBRT if your platform requires linking with librt (glibc version # before 2.17) for clock_gettime and CLOCK_MONOTONIC. # @@ -1881,6 +1883,10 @@ ifdef HAVE_CLOCK_MONOTONIC BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC endif +ifdef HAVE_SYNC_FILE_RANGE + BASIC_CFLAGS += -DHAVE_SYNC_FILE_RANGE +endif + ifdef NEEDS_LIBRT EXTLIBS += -lrt endif diff --git a/cache.h b/cache.h index eba12487b99..9cd60d94952 100644 --- a/cache.h +++ b/cache.h @@ -986,6 +986,13 @@ extern int read_replace_refs; extern char *git_replace_ref_base; extern int fsync_object_files; + +enum fsync_method { + FSYNC_METHOD_FSYNC, + FSYNC_METHOD_WRITEOUT_ONLY +}; + +extern enum fsync_method fsync_method; extern int core_preload_index; extern int precomposed_unicode; extern int protect_hfs; diff --git a/compat/mingw.h b/compat/mingw.h index c9a52ad64a6..6074a3d3ced 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -329,6 +329,9 @@ int mingw_getpagesize(void); #define getpagesize mingw_getpagesize #endif +int win32_fsync_no_flush(int fd); +#define fsync_no_flush win32_fsync_no_flush + struct rlimit { unsigned int rlim_cur; }; diff --git a/compat/win32/flush.c b/compat/win32/flush.c new file mode 100644 index 00000000000..6ca82f1dae5 --- /dev/null +++ b/compat/win32/flush.c @@ -0,0 +1,28 @@ +#include "git-compat-util.h" +#include +#include "lazyload.h" + +int win32_fsync_no_flush(int fd) +{ + IO_STATUS_BLOCK io_status; + +#define FLUSH_FLAGS_FILE_DATA_ONLY 1 + + DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NtFlushBuffersFileEx, + HANDLE FileHandle, ULONG Flags, PVOID Parameters, ULONG ParameterSize, + PIO_STATUS_BLOCK IoStatusBlock); + + if (!INIT_PROC_ADDR(NtFlushBuffersFileEx)) { + errno = ENOSYS; + return -1; + } + + memset(&io_status, 0, sizeof(io_status)); + if (NtFlushBuffersFileEx((HANDLE)_get_osfhandle(fd), FLUSH_FLAGS_FILE_DATA_ONLY, + NULL, 0, &io_status)) { + errno = EINVAL; + return -1; + } + + return 0; +} diff --git a/config.c b/config.c index c5873f3a706..139df71ba17 100644 --- a/config.c +++ b/config.c @@ -1490,6 +1490,18 @@ static int git_default_core_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "core.fsyncmethod")) { + if (!value) + return config_error_nonbool(var); + if (!strcmp(value, "fsync")) + fsync_method = FSYNC_METHOD_FSYNC; + else if (!strcmp(value, "writeout-only")) + fsync_method = FSYNC_METHOD_WRITEOUT_ONLY; + else + warning(_("ignoring unknown core.fsyncMethod value '%s'"), value); + + } + if (!strcmp(var, "core.fsyncobjectfiles")) { fsync_object_files = git_config_bool(var, value); return 0; diff --git a/config.mak.uname b/config.mak.uname index d0701f9beb0..774a09622d2 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -57,6 +57,7 @@ ifeq ($(uname_S),Linux) HAVE_CLOCK_MONOTONIC = YesPlease # -lrt is needed for clock_gettime on glibc <= 2.16 NEEDS_LIBRT = YesPlease + HAVE_SYNC_FILE_RANGE = YesPlease HAVE_GETDELIM = YesPlease FREAD_READS_DIRECTORIES = UnfortunatelyYes BASIC_CFLAGS += -DHAVE_SYSINFO @@ -453,6 +454,7 @@ endif CFLAGS = BASIC_CFLAGS = -nologo -I. -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ + compat/win32/flush.o \ compat/win32/path-utils.o \ compat/win32/pthread.o compat/win32/syslog.o \ compat/win32/trace2_win32_process_info.o \ @@ -628,6 +630,7 @@ ifeq ($(uname_S),MINGW) COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\" COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/trace2_win32_process_info.o \ + compat/win32/flush.o \ compat/win32/path-utils.o \ compat/win32/pthread.o compat/win32/syslog.o \ compat/win32/dirent.o diff --git a/configure.ac b/configure.ac index 5ee25ec95c8..6bd6bef1c44 100644 --- a/configure.ac +++ b/configure.ac @@ -1082,6 +1082,14 @@ AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC], [AC_MSG_RESULT([no]) HAVE_CLOCK_MONOTONIC=]) GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC]) + +# +# Define HAVE_SYNC_FILE_RANGE=YesPlease if sync_file_range is available. +GIT_CHECK_FUNC(sync_file_range, + [HAVE_SYNC_FILE_RANGE=YesPlease], + [HAVE_SYNC_FILE_RANGE]) +GIT_CONF_SUBST([HAVE_SYNC_FILE_RANGE]) + # # Define NO_SETITIMER if you don't have setitimer. GIT_CHECK_FUNC(setitimer, diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 86b46114464..6d7bc16d054 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -261,7 +261,8 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows") NOGDI OBJECT_CREATION_MODE=1 __USE_MINGW_ANSI_STDIO=0 USE_NED_ALLOCATOR OVERRIDE_STRDUP MMAP_PREVENTS_DELETE USE_WIN32_MMAP UNICODE _UNICODE HAVE_WPGMPTR ENSURE_MSYSTEM_IS_SET) - list(APPEND compat_SOURCES compat/mingw.c compat/winansi.c compat/win32/path-utils.c + list(APPEND compat_SOURCES compat/mingw.c compat/winansi.c + compat/win32/flush.c compat/win32/path-utils.c compat/win32/pthread.c compat/win32mmap.c compat/win32/syslog.c compat/win32/trace2_win32_process_info.c compat/win32/dirent.c compat/nedmalloc/nedmalloc.c compat/strdup.c) diff --git a/environment.c b/environment.c index 9da7f3c1a19..f9140e842cf 100644 --- a/environment.c +++ b/environment.c @@ -41,7 +41,7 @@ const char *git_attributes_file; const char *git_hooks_path; int zlib_compression_level = Z_BEST_SPEED; int pack_compression_level = Z_DEFAULT_COMPRESSION; -int fsync_object_files; +enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT; size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; size_t delta_base_cache_limit = 96 * 1024 * 1024; diff --git a/git-compat-util.h b/git-compat-util.h index c6bd2a84e55..50db85a8610 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1239,6 +1239,30 @@ __attribute__((format (printf, 1, 2))) NORETURN void BUG(const char *fmt, ...); #endif +#ifdef __APPLE__ +#define FSYNC_METHOD_DEFAULT FSYNC_METHOD_WRITEOUT_ONLY +#else +#define FSYNC_METHOD_DEFAULT FSYNC_METHOD_FSYNC +#endif + +enum fsync_action { + FSYNC_WRITEOUT_ONLY, + FSYNC_HARDWARE_FLUSH +}; + +/* + * Issues an fsync against the specified file according to the specified mode. + * + * FSYNC_WRITEOUT_ONLY attempts to use interfaces available on some operating + * systems to flush the OS cache without issuing a flush command to the storage + * controller. If those interfaces are unavailable, the function fails with + * ENOSYS. + * + * FSYNC_HARDWARE_FLUSH does an OS writeout and hardware flush to ensure that + * changes are durable. It is not expected to fail. + */ +int git_fsync(int fd, enum fsync_action action); + /* * Preserves errno, prints a message, but gives no warning for ENOENT. * Returns 0 on success, which includes trying to unlink an object that does diff --git a/wrapper.c b/wrapper.c index 36e12119d76..572f28f14ff 100644 --- a/wrapper.c +++ b/wrapper.c @@ -546,6 +546,70 @@ int xmkstemp_mode(char *filename_template, int mode) return fd; } +/* + * Some platforms return EINTR from fsync. Since fsync is invoked in some + * cases by a wrapper that dies on failure, do not expose EINTR to callers. + */ +static int fsync_loop(int fd) +{ + int err; + + do { + err = fsync(fd); + } while (err < 0 && errno == EINTR); + return err; +} + +int git_fsync(int fd, enum fsync_action action) +{ + switch (action) { + case FSYNC_WRITEOUT_ONLY: + +#ifdef __APPLE__ + /* + * on macOS, fsync just causes filesystem cache writeback but does not + * flush hardware caches. + */ + return fsync_loop(fd); +#endif + +#ifdef HAVE_SYNC_FILE_RANGE + /* + * On linux 2.6.17 and above, sync_file_range is the way to issue + * a writeback without a hardware flush. An offset of 0 and size of 0 + * indicates writeout of the entire file and the wait flags ensure that all + * dirty data is written to the disk (potentially in a disk-side cache) + * before we continue. + */ + + return sync_file_range(fd, 0, 0, SYNC_FILE_RANGE_WAIT_BEFORE | + SYNC_FILE_RANGE_WRITE | + SYNC_FILE_RANGE_WAIT_AFTER); +#endif + +#ifdef fsync_no_flush + return fsync_no_flush(fd); +#endif + + errno = ENOSYS; + return -1; + + case FSYNC_HARDWARE_FLUSH: + /* + * On some platforms fsync may return EINTR. Try again in this + * case, since callers asking for a hardware flush may die if + * this function returns an error. + */ +#ifdef __APPLE__ + return fcntl(fd, F_FULLFSYNC); +#else + return fsync_loop(fd); +#endif + default: + BUG("unexpected git_fsync(%d) call", action); + } +} + static int warn_if_unremovable(const char *op, const char *file, int rc) { int err; diff --git a/write-or-die.c b/write-or-die.c index 0b1ec8190b6..0702acdd5e8 100644 --- a/write-or-die.c +++ b/write-or-die.c @@ -57,10 +57,12 @@ void fprintf_or_die(FILE *f, const char *fmt, ...) void fsync_or_die(int fd, const char *msg) { - while (fsync(fd) < 0) { - if (errno != EINTR) - die_errno("fsync error on '%s'", msg); - } + if (fsync_method == FSYNC_METHOD_WRITEOUT_ONLY && + git_fsync(fd, FSYNC_WRITEOUT_ONLY) >= 0) + return; + + if (git_fsync(fd, FSYNC_HARDWARE_FLUSH) < 0) + die_errno("fsync error on '%s'", msg); } void write_or_die(int fd, const void *buf, size_t count) From patchwork Thu Dec 9 00:57:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Neeraj Singh (WINDOWS-SFS)" X-Patchwork-Id: 12665635 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27AD7C433F5 for ; Thu, 9 Dec 2021 00:57:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241723AbhLIBA4 (ORCPT ); Wed, 8 Dec 2021 20:00:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45304 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241683AbhLIBAx (ORCPT ); Wed, 8 Dec 2021 20:00:53 -0500 Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 25F02C061746 for ; Wed, 8 Dec 2021 16:57:20 -0800 (PST) Received: by mail-wr1-x42b.google.com with SMTP id d9so7051030wrw.4 for ; Wed, 08 Dec 2021 16:57:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=OyIj+QLSMYS4QH2WFq0NHThTdQ8m3DE6fDnvHyxHqi4=; b=U4x/z8caZvNO/CLeCMuxsZ0I4382Fny9rcTfTAqQMP6NHI5Na+8hk3fOJrx3UwKaw1 Vdh6bbV86T795LhyCjGUJkDTC/EKLllMOn2sm09H5le2srRgSZqEvPGWD20BNo3zBbIf 6y5KNU9la69xfrMcIwEtIQdN/x/P1j5ub022rfZzUu9ePpGmBZBzNNWgnVT3PJmL/An9 2mqGOm1AnIyUHlkaHA/LnhCbgjsdOmiaO5z9sC2AjQE6nhcAxx4ZML5uynTUZ5MaVdMI MSLii72JAEqYJwyEtC78Vi8TnKOare+d62m8pvGve+E/fRK+2G3c+nZWwskXxUGGZqk9 S5kA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=OyIj+QLSMYS4QH2WFq0NHThTdQ8m3DE6fDnvHyxHqi4=; b=TsWK5V1nTlNA9CI5/VD9i0okCEtr31mMOTR+MkxoWdvpWREjciJyqQX5e5SYcqvcW7 0sXAmzjkF3FYyH7RKfKvN3U7lAMfhct8BHRh/UOqKCcpsOr+fZmVLoe0hdxU0dK5UwCo 1q4fdbxqWavEMS9xBMg+rTqffpfDRdfKC3+xAiYQ84Ac7PaD2OcTnAr2BrM0hCD07K/x jdIH5AZAtNf0+PntXal2z/luh3XzZvT/4+OCG7VySjqURJ6mGg5RYvAz8LJTn5R2gt22 rg6+HbGlgDkbPbfMKS9sXmwhDAj9mK79fr41y+nnsPaX5KTA0RhFZv5ng8h3D0jsZZl9 JICQ== X-Gm-Message-State: AOAM533qtZHrncTlCXmqEZuzCuSXdjACiQgl7bFgyL35LuX75o4bVA44 jryTT8RnGdfUUe6+MHWclaflhO4CDNc= X-Google-Smtp-Source: ABdhPJwQsMkH0bMkl76nx6pSni1jY4wyMEfXAOxg9Futv+kkveB0PvGvn3p7Xc2xw3GfpkMabr9Vcw== X-Received: by 2002:adf:e484:: with SMTP id i4mr2572657wrm.49.1639011438245; Wed, 08 Dec 2021 16:57:18 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l4sm4074674wrv.94.2021.12.08.16.57.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Dec 2021 16:57:17 -0800 (PST) Message-Id: <080be1a6f642c64b5cbd74c612027245026833d3.1639011434.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 09 Dec 2021 00:57:11 +0000 Subject: [PATCH v3 2/4] core.fsync: introduce granular fsync control Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: rsbecker@nexbridge.com, bagasdotme@gmail.com, newren@gmail.com, avarab@gmail.com, nksingh85@gmail.com, ps@pks.im, "Neeraj K. Singh" , Neeraj Singh Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Neeraj Singh From: Neeraj Singh This commit introduces the `core.fsync` configuration knob which can be used to control how components of the repository are made durable on disk. This setting allows future extensibility of the list of syncable components: * We issue a warning rather than an error for unrecognized components, so new configs can be used with old Git versions. * We support negation, so users can choose one of the default aggregate options and then remove components that they don't want. The user would then harden any new components added in a Git version update. This also supports the common request of doing absolutely no fysncing with the `core.fsync=none` value, which is expected to make the test suite faster. Signed-off-by: Neeraj Singh --- Documentation/config/core.txt | 27 +++++++++---- builtin/fast-import.c | 2 +- builtin/index-pack.c | 4 +- builtin/pack-objects.c | 8 ++-- bulk-checkin.c | 5 ++- cache.h | 39 +++++++++++++++++- commit-graph.c | 3 +- config.c | 76 ++++++++++++++++++++++++++++++++++- csum-file.c | 5 ++- csum-file.h | 3 +- environment.c | 1 + midx.c | 3 +- object-file.c | 3 +- pack-bitmap-write.c | 3 +- pack-write.c | 13 +++--- read-cache.c | 2 +- 16 files changed, 164 insertions(+), 33 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index dbb134f7136..4f1747ec871 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -547,6 +547,25 @@ core.whitespace:: is relevant for `indent-with-non-tab` and when Git fixes `tab-in-indent` errors. The default tab width is 8. Allowed values are 1 to 63. +core.fsync:: + A comma-separated list of parts of the repository which should be + hardened via the core.fsyncMethod when created or modified. You can + disable hardening of any component by prefixing it with a '-'. Later + items take precedence over earlier ones in the list. For example, + `core.fsync=all,-pack-metadata` means "harden everything except pack + metadata." Items that are not hardened may be lost in the event of an + unclean system shutdown. ++ +* `none` disables fsync completely. This must be specified alone. +* `loose-object` hardens objects added to the repo in loose-object form. +* `pack` hardens objects added to the repo in packfile form. +* `pack-metadata` hardens packfile bitmaps and indexes. +* `commit-graph` hardens the commit graph file. +* `objects` is an aggregate option that includes `loose-objects`, `pack`, + `pack-metadata`, and `commit-graph`. +* `default` is an aggregate option that is equivalent to `objects,-loose-object` +* `all` is an aggregate option that syncs all individual components above. + core.fsyncMethod:: A value indicating the strategy Git will use to harden repository data using fsync and related primitives. @@ -556,14 +575,6 @@ core.fsyncMethod:: filesystem and storage hardware, data added to the repository may not be durable in the event of a system crash. This is the default mode on macOS. -core.fsyncObjectFiles:: - This boolean will enable 'fsync()' when writing object files. -+ -This is a total waste of time and effort on a filesystem that orders -data writes properly, but can be useful for filesystems that do not use -journalling (traditional UNIX filesystems) or that only journal metadata -and not file contents (OS X's HFS+, or Linux ext3 with "data=writeback"). - core.preloadIndex:: Enable parallel index preload for operations like 'git diff' + diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 20406f67754..e27a4580f85 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -856,7 +856,7 @@ static void end_packfile(void) struct tag *t; close_pack_windows(pack_data); - finalize_hashfile(pack_file, cur_pack_oid.hash, 0); + finalize_hashfile(pack_file, cur_pack_oid.hash, FSYNC_COMPONENT_PACK, 0); fixup_pack_header_footer(pack_data->pack_fd, pack_data->hash, pack_data->pack_name, object_count, cur_pack_oid.hash, pack_size); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index c23d01de7dc..c32534c13b4 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1286,7 +1286,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha nr_objects - nr_objects_initial); stop_progress_msg(&progress, msg.buf); strbuf_release(&msg); - finalize_hashfile(f, tail_hash, 0); + finalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0); hashcpy(read_hash, pack_hash); fixup_pack_header_footer(output_fd, pack_hash, curr_pack, nr_objects, @@ -1508,7 +1508,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name, if (!from_stdin) { close(input_fd); } else { - fsync_or_die(output_fd, curr_pack_name); + fsync_component_or_die(FSYNC_COMPONENT_PACK, output_fd, curr_pack_name); err = close(output_fd); if (err) die_errno(_("error while closing pack file")); diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 857be7826f3..916c55d6ce9 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1204,11 +1204,13 @@ static void write_pack_file(void) * If so, rewrite it like in fast-import */ if (pack_to_stdout) { - finalize_hashfile(f, hash, CSUM_HASH_IN_STREAM | CSUM_CLOSE); + finalize_hashfile(f, hash, FSYNC_COMPONENT_NONE, + CSUM_HASH_IN_STREAM | CSUM_CLOSE); } else if (nr_written == nr_remaining) { - finalize_hashfile(f, hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); + finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); } else { - int fd = finalize_hashfile(f, hash, 0); + int fd = finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, 0); fixup_pack_header_footer(fd, hash, pack_tmp_name, nr_written, hash, offset); close(fd); diff --git a/bulk-checkin.c b/bulk-checkin.c index 8785b2ac806..a2cf9dcbc8d 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -53,9 +53,10 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state) unlink(state->pack_tmp_name); goto clear_exit; } else if (state->nr_written == 1) { - finalize_hashfile(state->f, hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); + finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); } else { - int fd = finalize_hashfile(state->f, hash, 0); + int fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0); fixup_pack_header_footer(fd, hash, state->pack_tmp_name, state->nr_written, hash, state->offset); diff --git a/cache.h b/cache.h index 9cd60d94952..d83fbaf2619 100644 --- a/cache.h +++ b/cache.h @@ -985,7 +985,38 @@ void reset_shared_repository(void); extern int read_replace_refs; extern char *git_replace_ref_base; -extern int fsync_object_files; +/* + * These values are used to help identify parts of a repository to fsync. + * FSYNC_COMPONENT_NONE identifies data that will not be a persistent part of the + * repository and so shouldn't be fsynced. + */ +enum fsync_component { + FSYNC_COMPONENT_NONE = 0, + FSYNC_COMPONENT_LOOSE_OBJECT = 1 << 0, + FSYNC_COMPONENT_PACK = 1 << 1, + FSYNC_COMPONENT_PACK_METADATA = 1 << 2, + FSYNC_COMPONENT_COMMIT_GRAPH = 1 << 3, +}; + +#define FSYNC_COMPONENTS_DEFAULT (FSYNC_COMPONENT_PACK | \ + FSYNC_COMPONENT_PACK_METADATA | \ + FSYNC_COMPONENT_COMMIT_GRAPH) + +#define FSYNC_COMPONENTS_OBJECTS (FSYNC_COMPONENT_LOOSE_OBJECT | \ + FSYNC_COMPONENT_PACK | \ + FSYNC_COMPONENT_PACK_METADATA | \ + FSYNC_COMPONENT_COMMIT_GRAPH) + +#define FSYNC_COMPONENTS_ALL (FSYNC_COMPONENT_LOOSE_OBJECT | \ + FSYNC_COMPONENT_PACK | \ + FSYNC_COMPONENT_PACK_METADATA | \ + FSYNC_COMPONENT_COMMIT_GRAPH) + + +/* + * A bitmask indicating which components of the repo should be fsynced. + */ +extern enum fsync_component fsync_components; enum fsync_method { FSYNC_METHOD_FSYNC, @@ -1747,6 +1778,12 @@ int copy_file_with_time(const char *dst, const char *src, int mode); void write_or_die(int fd, const void *buf, size_t count); void fsync_or_die(int fd, const char *); +inline void fsync_component_or_die(enum fsync_component component, int fd, const char *msg) +{ + if (fsync_components & component) + fsync_or_die(fd, msg); +} + ssize_t read_in_full(int fd, void *buf, size_t count); ssize_t write_in_full(int fd, const void *buf, size_t count); ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset); diff --git a/commit-graph.c b/commit-graph.c index 2706683acfe..c8a5dea4541 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1939,7 +1939,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } close_commit_graph(ctx->r->objects); - finalize_hashfile(f, file_hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC); + finalize_hashfile(f, file_hash, FSYNC_COMPONENT_COMMIT_GRAPH, + CSUM_HASH_IN_STREAM | CSUM_FSYNC); free_chunkfile(cf); if (ctx->split) { diff --git a/config.c b/config.c index 139df71ba17..5ab381388f9 100644 --- a/config.c +++ b/config.c @@ -1213,6 +1213,73 @@ static int git_parse_maybe_bool_text(const char *value) return -1; } +static const struct fsync_component_entry { + const char *name; + enum fsync_component component_bits; +} fsync_component_table[] = { + { "loose-object", FSYNC_COMPONENT_LOOSE_OBJECT }, + { "pack", FSYNC_COMPONENT_PACK }, + { "pack-metadata", FSYNC_COMPONENT_PACK_METADATA }, + { "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH }, + { "objects", FSYNC_COMPONENTS_OBJECTS }, + { "default", FSYNC_COMPONENTS_DEFAULT }, + { "all", FSYNC_COMPONENTS_ALL }, +}; + +static enum fsync_component parse_fsync_components(const char *var, const char *string) +{ + enum fsync_component output = 0; + + if (!strcmp(string, "none")) + return output; + + while (string) { + int i; + size_t len; + const char *ep; + int negated = 0; + int found = 0; + + string = string + strspn(string, ", \t\n\r"); + ep = strchrnul(string, ','); + len = ep - string; + + if (*string == '-') { + negated = 1; + string++; + len--; + if (!len) + warning(_("invalid value for variable %s"), var); + } + + if (!len) + break; + + for (i = 0; i < ARRAY_SIZE(fsync_component_table); ++i) { + const struct fsync_component_entry *entry = &fsync_component_table[i]; + + if (strncmp(entry->name, string, len)) + continue; + + found = 1; + if (negated) + output &= ~entry->component_bits; + else + output |= entry->component_bits; + } + + if (!found) { + char *component = xstrndup(string, len); + warning(_("ignoring unknown core.fsync component '%s'"), component); + free(component); + } + + string = ep; + } + + return output; +} + int git_parse_maybe_bool(const char *value) { int v = git_parse_maybe_bool_text(value); @@ -1490,6 +1557,13 @@ static int git_default_core_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "core.fsync")) { + if (!value) + return config_error_nonbool(var); + fsync_components = parse_fsync_components(var, value); + return 0; + } + if (!strcmp(var, "core.fsyncmethod")) { if (!value) return config_error_nonbool(var); @@ -1503,7 +1577,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "core.fsyncobjectfiles")) { - fsync_object_files = git_config_bool(var, value); + warning(_("core.fsyncobjectfiles is deprecated; use core.fsync instead")); return 0; } diff --git a/csum-file.c b/csum-file.c index 26e8a6df44e..59ef3398ca2 100644 --- a/csum-file.c +++ b/csum-file.c @@ -58,7 +58,8 @@ static void free_hashfile(struct hashfile *f) free(f); } -int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int flags) +int finalize_hashfile(struct hashfile *f, unsigned char *result, + enum fsync_component component, unsigned int flags) { int fd; @@ -69,7 +70,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int fl if (flags & CSUM_HASH_IN_STREAM) flush(f, f->buffer, the_hash_algo->rawsz); if (flags & CSUM_FSYNC) - fsync_or_die(f->fd, f->name); + fsync_component_or_die(component, f->fd, f->name); if (flags & CSUM_CLOSE) { if (close(f->fd)) die_errno("%s: sha1 file error on close", f->name); diff --git a/csum-file.h b/csum-file.h index 291215b34eb..0d29f528fbc 100644 --- a/csum-file.h +++ b/csum-file.h @@ -1,6 +1,7 @@ #ifndef CSUM_FILE_H #define CSUM_FILE_H +#include "cache.h" #include "hash.h" struct progress; @@ -38,7 +39,7 @@ int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *); struct hashfile *hashfd(int fd, const char *name); struct hashfile *hashfd_check(const char *name); struct hashfile *hashfd_throughput(int fd, const char *name, struct progress *tp); -int finalize_hashfile(struct hashfile *, unsigned char *, unsigned int); +int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int); void hashwrite(struct hashfile *, const void *, unsigned int); void hashflush(struct hashfile *f); void crc32_begin(struct hashfile *); diff --git a/environment.c b/environment.c index f9140e842cf..09905adecf9 100644 --- a/environment.c +++ b/environment.c @@ -42,6 +42,7 @@ const char *git_hooks_path; int zlib_compression_level = Z_BEST_SPEED; int pack_compression_level = Z_DEFAULT_COMPRESSION; enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT; +enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT; size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; size_t delta_base_cache_limit = 96 * 1024 * 1024; diff --git a/midx.c b/midx.c index 837b46b2af5..882f91f7d57 100644 --- a/midx.c +++ b/midx.c @@ -1406,7 +1406,8 @@ static int write_midx_internal(const char *object_dir, write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs); write_chunkfile(cf, &ctx); - finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM); + finalize_hashfile(f, midx_hash, FSYNC_COMPONENT_PACK_METADATA, + CSUM_FSYNC | CSUM_HASH_IN_STREAM); free_chunkfile(cf); if (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP)) diff --git a/object-file.c b/object-file.c index eb972cdccd2..9d9c4a39e85 100644 --- a/object-file.c +++ b/object-file.c @@ -1809,8 +1809,7 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf, /* Finalize a file on disk, and close it. */ static void close_loose_object(int fd) { - if (fsync_object_files) - fsync_or_die(fd, "loose object file"); + fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd, "loose object file"); if (close(fd) != 0) die_errno(_("error when closing loose object file")); } diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 9c55c1531e1..c16e43d1669 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -719,7 +719,8 @@ void bitmap_writer_finish(struct pack_idx_entry **index, if (options & BITMAP_OPT_HASH_CACHE) write_hash_cache(f, index, index_nr); - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA, + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); if (adjust_shared_perm(tmp_file.buf)) die_errno("unable to make temporary bitmap file readable"); diff --git a/pack-write.c b/pack-write.c index a5846f3a346..51812cb1299 100644 --- a/pack-write.c +++ b/pack-write.c @@ -159,9 +159,9 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec } hashwrite(f, sha1, the_hash_algo->rawsz); - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE | - ((opts->flags & WRITE_IDX_VERIFY) - ? 0 : CSUM_FSYNC)); + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA, + CSUM_HASH_IN_STREAM | CSUM_CLOSE | + ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC)); return index_name; } @@ -281,8 +281,9 @@ const char *write_rev_file_order(const char *rev_name, if (rev_name && adjust_shared_perm(rev_name) < 0) die(_("failed to make %s readable"), rev_name); - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE | - ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC)); + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA, + CSUM_HASH_IN_STREAM | CSUM_CLOSE | + ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC)); return rev_name; } @@ -390,7 +391,7 @@ void fixup_pack_header_footer(int pack_fd, the_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx); the_hash_algo->final_fn(new_pack_hash, &new_hash_ctx); write_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz); - fsync_or_die(pack_fd, pack_name); + fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name); } char *index_pack_lockfile(int ip_out, int *is_well_formed) diff --git a/read-cache.c b/read-cache.c index f3986596623..f3539681f49 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3060,7 +3060,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, return -1; } - finalize_hashfile(f, istate->oid.hash, CSUM_HASH_IN_STREAM); + finalize_hashfile(f, istate->oid.hash, FSYNC_COMPONENT_NONE, CSUM_HASH_IN_STREAM); if (close_tempfile_gently(tempfile)) { error(_("could not close '%s'"), get_tempfile_path(tempfile)); return -1; From patchwork Thu Dec 9 00:57:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Neeraj Singh (WINDOWS-SFS)" X-Patchwork-Id: 12665637 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 249F3C433FE for ; Thu, 9 Dec 2021 00:57:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241736AbhLIBA5 (ORCPT ); Wed, 8 Dec 2021 20:00:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45308 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241735AbhLIBAx (ORCPT ); Wed, 8 Dec 2021 20:00:53 -0500 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1ECAEC0617A2 for ; Wed, 8 Dec 2021 16:57:21 -0800 (PST) Received: by mail-wm1-x333.google.com with SMTP id m25-20020a7bcb99000000b0033aa12cdd33so4319888wmi.1 for ; Wed, 08 Dec 2021 16:57:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=FTWJHoSVmwxYnnW3XFe39EGz1hNwR2AP/w3YE/tC71E=; b=EO1zHrHirPYM7URETKQr6ijuJg24FnYqyDMATl/rTWD0aM0pZVZx1tnMXGAVv/Ym6q 3fHFJ8/MBjXUG7WZJHlbRSDxL++/fgBzEWvz/ndeZ1UGZpA8QYxf0mwFQIY16eKODfn+ RkwSKrDChlY16lDL0E1VTV9tR8iCCx2nM7VMz00XJywDPnng8Jt5yY2DuJkQxUzK93cg 0BCUmOPab5YRr1uQJ5XYdEgMxxCX7yGgtG+IxGpOr+jV1Rgji0FFlbLC3FMzmJFvfYg4 XT7XO/XZDrmOfTym03Vc3by7WS5kUKxeUye+QLlaNxY+vjO1dBgqfnhtK5INPXTwiHkc o5FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=FTWJHoSVmwxYnnW3XFe39EGz1hNwR2AP/w3YE/tC71E=; b=rTTqGxATjehvEot3hAbaRgzD28MndMtOV3qvxU5wsm2a5KO9o3yczO1FsDWSl2liL6 dKCretj6PGoCt1mQQMZQAPEknfiq9hrCSc6eBAqWRofVxsNJHznxBs1mE7Rb33Hyqlw7 CntcjLzXXjVMgLDVFtZTYqRA1UTgvTtm1wheozi3wChdPYGXA32LPOY5UX+rAWnJeZDR xvWgL1RO48cumES9Li2Tgowj8mj8msyJZqDXPOq2Re16xNI5peeHLj1mBAx9vnCvM/rk ZdRjgBgz331oQC6MKfvkieZqWf40D/0t1lGHxfkpXP1O4dvB+PyO859aU7QxEO8bd2bD 6y+A== X-Gm-Message-State: AOAM531jfaJ35zESUCtP7gswN/yDKBlxIRSZypoYtEBRPPBSf/ISc/Iy 7gglyG2l+qtBu3tweBlm4tXnqrHg+yc= X-Google-Smtp-Source: ABdhPJzq/nUz38wDKgmjxlaxPNv3ogOdjbXhRFN/FssTsWUJ+hTScCYOjE0h5fov/qSLFDmKhPHsVA== X-Received: by 2002:a7b:c85a:: with SMTP id c26mr2973013wml.23.1639011439557; Wed, 08 Dec 2021 16:57:19 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 10sm5526937wrb.75.2021.12.08.16.57.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Dec 2021 16:57:18 -0800 (PST) Message-Id: <2207950beba89b690690f98c77761c27c5da8dcc.1639011434.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 09 Dec 2021 00:57:12 +0000 Subject: [PATCH v3 3/4] core.fsync: new option to harden the index Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: rsbecker@nexbridge.com, bagasdotme@gmail.com, newren@gmail.com, avarab@gmail.com, nksingh85@gmail.com, ps@pks.im, "Neeraj K. Singh" , Neeraj Singh Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Neeraj Singh From: Neeraj Singh This commit introduces the new ability for the user to harden the index. In the event of a system crash, the index must be durable for the user to actually find a file that has been added to the repo and then deleted from the working tree. We use the presence of the COMMIT_LOCK flag and absence of the alternate_index_output as a proxy for determining whether we're updating the persistent index of the repo or some temporary index. We don't sync these temporary indexes. Signed-off-by: Neeraj Singh --- Documentation/config/core.txt | 1 + cache.h | 4 +++- config.c | 1 + read-cache.c | 19 +++++++++++++------ 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index 4f1747ec871..8e5b7a795ab 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -561,6 +561,7 @@ core.fsync:: * `pack` hardens objects added to the repo in packfile form. * `pack-metadata` hardens packfile bitmaps and indexes. * `commit-graph` hardens the commit graph file. +* `index` hardens the index when it is modified. * `objects` is an aggregate option that includes `loose-objects`, `pack`, `pack-metadata`, and `commit-graph`. * `default` is an aggregate option that is equivalent to `objects,-loose-object` diff --git a/cache.h b/cache.h index d83fbaf2619..4dc26d7b2c9 100644 --- a/cache.h +++ b/cache.h @@ -996,6 +996,7 @@ enum fsync_component { FSYNC_COMPONENT_PACK = 1 << 1, FSYNC_COMPONENT_PACK_METADATA = 1 << 2, FSYNC_COMPONENT_COMMIT_GRAPH = 1 << 3, + FSYNC_COMPONENT_INDEX = 1 << 4, }; #define FSYNC_COMPONENTS_DEFAULT (FSYNC_COMPONENT_PACK | \ @@ -1010,7 +1011,8 @@ enum fsync_component { #define FSYNC_COMPONENTS_ALL (FSYNC_COMPONENT_LOOSE_OBJECT | \ FSYNC_COMPONENT_PACK | \ FSYNC_COMPONENT_PACK_METADATA | \ - FSYNC_COMPONENT_COMMIT_GRAPH) + FSYNC_COMPONENT_COMMIT_GRAPH | \ + FSYNC_COMPONENT_INDEX) /* diff --git a/config.c b/config.c index 5ab381388f9..b3e7006c68e 100644 --- a/config.c +++ b/config.c @@ -1221,6 +1221,7 @@ static const struct fsync_component_entry { { "pack", FSYNC_COMPONENT_PACK }, { "pack-metadata", FSYNC_COMPONENT_PACK_METADATA }, { "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH }, + { "index", FSYNC_COMPONENT_INDEX }, { "objects", FSYNC_COMPONENTS_OBJECTS }, { "default", FSYNC_COMPONENTS_DEFAULT }, { "all", FSYNC_COMPONENTS_ALL }, diff --git a/read-cache.c b/read-cache.c index f3539681f49..783cb3ea5db 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2816,7 +2816,7 @@ static int record_ieot(void) * rely on it. */ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, - int strip_extensions) + int strip_extensions, unsigned flags) { uint64_t start = getnanotime(); struct hashfile *f; @@ -2830,6 +2830,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, struct strbuf previous_name_buf = STRBUF_INIT, *previous_name; int drop_cache_tree = istate->drop_cache_tree; off_t offset; + int csum_fsync_flag; int ieot_entries = 1; struct index_entry_offset_table *ieot = NULL; int nr, nr_threads; @@ -3060,7 +3061,13 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, return -1; } - finalize_hashfile(f, istate->oid.hash, FSYNC_COMPONENT_NONE, CSUM_HASH_IN_STREAM); + csum_fsync_flag = 0; + if (!alternate_index_output && (flags & COMMIT_LOCK)) + csum_fsync_flag = CSUM_FSYNC; + + finalize_hashfile(f, istate->oid.hash, FSYNC_COMPONENT_INDEX, + CSUM_HASH_IN_STREAM | csum_fsync_flag); + if (close_tempfile_gently(tempfile)) { error(_("could not close '%s'"), get_tempfile_path(tempfile)); return -1; @@ -3115,7 +3122,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l */ trace2_region_enter_printf("index", "do_write_index", the_repository, "%s", get_lock_file_path(lock)); - ret = do_write_index(istate, lock->tempfile, 0); + ret = do_write_index(istate, lock->tempfile, 0, flags); trace2_region_leave_printf("index", "do_write_index", the_repository, "%s", get_lock_file_path(lock)); @@ -3209,7 +3216,7 @@ static int clean_shared_index_files(const char *current_hex) } static int write_shared_index(struct index_state *istate, - struct tempfile **temp) + struct tempfile **temp, unsigned flags) { struct split_index *si = istate->split_index; int ret, was_full = !istate->sparse_index; @@ -3219,7 +3226,7 @@ static int write_shared_index(struct index_state *istate, trace2_region_enter_printf("index", "shared/do_write_index", the_repository, "%s", get_tempfile_path(*temp)); - ret = do_write_index(si->base, *temp, 1); + ret = do_write_index(si->base, *temp, 1, flags); trace2_region_leave_printf("index", "shared/do_write_index", the_repository, "%s", get_tempfile_path(*temp)); @@ -3328,7 +3335,7 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock, ret = do_write_locked_index(istate, lock, flags); goto out; } - ret = write_shared_index(istate, &temp); + ret = write_shared_index(istate, &temp, flags); saved_errno = errno; if (is_tempfile_active(temp)) From patchwork Thu Dec 9 00:57:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Neeraj Singh (WINDOWS-SFS)" X-Patchwork-Id: 12665639 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D498C4332F for ; Thu, 9 Dec 2021 00:57:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241769AbhLIBA6 (ORCPT ); Wed, 8 Dec 2021 20:00:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45316 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241744AbhLIBAz (ORCPT ); Wed, 8 Dec 2021 20:00:55 -0500 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 42B97C061A32 for ; Wed, 8 Dec 2021 16:57:22 -0800 (PST) Received: by mail-wr1-x436.google.com with SMTP id t9so7044674wrx.7 for ; Wed, 08 Dec 2021 16:57:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=sZ4d+Vc6TZf1kygLr+gzSqna+teDkJh+PKnLy1fAYtE=; b=C/BduWJpu9iU8hoUuXCOMQ8IOcf/MclI11emxj8kRjaVLd9kY9MnCyFXUfqd9YvJdd iBZxK2Bn0c52TaKhpwl2UlQ+NsmOmEGkIjnByl+HGRM5/RJkkDkokGFzrVaEiCpE+2Fm mF6omU1/AKiDXZYKSb/f5GjFBNqCbvSUk5ZHnz+E5my04xADdhZKvYC/ejesiERAM5NF PdN/TrIjTxXhh4k5v+zNOx3DxgivqTiSpd2uZcbzkUgG1uz+lKvMrKmgRtzjln2oYOE3 1FNJuMeZ30sEovF66x6RpPOV8ZlIteBKFebC8icWI5my7OFZeHqHBARZs73JZqSyo4F3 x1CA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=sZ4d+Vc6TZf1kygLr+gzSqna+teDkJh+PKnLy1fAYtE=; b=hYmYjiHQmLhRnilMJZ5BbN07Xo/W0+d3CZB26FLIBXHjAIQnYh3iabdf4SMiorT091 ISXx4PrIDzMRXe/VcBKF8zZOf4lhnShbTyyoxwLlSXLkzvXeEryjW6osigWdj/Ed4WN3 t7Tk4ijoHb3BbtDhkofVu7x/78oAkOfHZsXGnowbO8SI3wc6WGNaIEc3ie91uuICb7jG AGwK6kgxA9Vgw2vC8BFR6mtQZ3jhliB48WYtE9A7ye13f88TqkxGd94uc09J44pP/al/ F5ViiAQ9/54b0rnxpivRo6bXDsL28QT9QqrWVIj+/yIKE0owN1Kom6uRr8l3EMO3ev9r xxJQ== X-Gm-Message-State: AOAM530kSn+IN//ar6Gm8eKruglC3cIXotemg8FStkoDC/BAnBV5FIIk 6JJWtPXylgYFRCnkwFDnDCs6bjdmyrM= X-Google-Smtp-Source: ABdhPJzT15+HR0Eg33AJXKy7QE7qr+KuRv9iPleroGI2ZWA0get5dkQRRT+nKtlHp0949jeycV1eDA== X-Received: by 2002:a5d:460d:: with SMTP id t13mr2563371wrq.44.1639011440628; Wed, 08 Dec 2021 16:57:20 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id b10sm4040527wrt.36.2021.12.08.16.57.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Dec 2021 16:57:19 -0800 (PST) Message-Id: In-Reply-To: References: Date: Thu, 09 Dec 2021 00:57:13 +0000 Subject: [PATCH v3 4/4] core.fsync: add a `derived-metadata` aggregate option Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: rsbecker@nexbridge.com, bagasdotme@gmail.com, newren@gmail.com, avarab@gmail.com, nksingh85@gmail.com, ps@pks.im, "Neeraj K. Singh" , Neeraj Singh Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Neeraj Singh From: Neeraj Singh This commit adds an aggregate option that currently includes the commit-graph file and pack metadata (indexes and bitmaps). The user may want to exclude this set from durability since they can be recomputed from other data if they wind up corrupt or missing. This is split out from the other patches in the series since it is an optional nice-to-have that might be controversial. Signed-off-by: Neeraj Singh --- Documentation/config/core.txt | 6 +++--- cache.h | 7 ++++--- config.c | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index 8e5b7a795ab..21092f3a4d1 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -562,9 +562,9 @@ core.fsync:: * `pack-metadata` hardens packfile bitmaps and indexes. * `commit-graph` hardens the commit graph file. * `index` hardens the index when it is modified. -* `objects` is an aggregate option that includes `loose-objects`, `pack`, - `pack-metadata`, and `commit-graph`. -* `default` is an aggregate option that is equivalent to `objects,-loose-object` +* `objects` is an aggregate option that includes `loose-objects` and `pack`. +* `derived-metadata` is an aggregate option that includes `pack-metadata` and `commit-graph`. +* `default` is an aggregate option that is equivalent to `objects,derived-metadata,-loose-object` * `all` is an aggregate option that syncs all individual components above. core.fsyncMethod:: diff --git a/cache.h b/cache.h index 4dc26d7b2c9..cc1c084242e 100644 --- a/cache.h +++ b/cache.h @@ -1004,9 +1004,10 @@ enum fsync_component { FSYNC_COMPONENT_COMMIT_GRAPH) #define FSYNC_COMPONENTS_OBJECTS (FSYNC_COMPONENT_LOOSE_OBJECT | \ - FSYNC_COMPONENT_PACK | \ - FSYNC_COMPONENT_PACK_METADATA | \ - FSYNC_COMPONENT_COMMIT_GRAPH) + FSYNC_COMPONENT_PACK) + +#define FSYNC_COMPONENTS_DERIVED_METADATA (FSYNC_COMPONENT_PACK_METADATA | \ + FSYNC_COMPONENT_COMMIT_GRAPH) #define FSYNC_COMPONENTS_ALL (FSYNC_COMPONENT_LOOSE_OBJECT | \ FSYNC_COMPONENT_PACK | \ diff --git a/config.c b/config.c index b3e7006c68e..d9ef3ef0060 100644 --- a/config.c +++ b/config.c @@ -1223,6 +1223,7 @@ static const struct fsync_component_entry { { "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH }, { "index", FSYNC_COMPONENT_INDEX }, { "objects", FSYNC_COMPONENTS_OBJECTS }, + { "derived-metadata", FSYNC_COMPONENTS_DERIVED_METADATA }, { "default", FSYNC_COMPONENTS_DEFAULT }, { "all", FSYNC_COMPONENTS_ALL }, };