From patchwork Tue Dec 7 02:46:49 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: 12660869 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 E3116C433EF for ; Tue, 7 Dec 2021 02:46:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230146AbhLGCu0 (ORCPT ); Mon, 6 Dec 2021 21:50:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51088 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229577AbhLGCuY (ORCPT ); Mon, 6 Dec 2021 21:50:24 -0500 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C6222C061354 for ; Mon, 6 Dec 2021 18:46:54 -0800 (PST) Received: by mail-wm1-x32f.google.com with SMTP id c6-20020a05600c0ac600b0033c3aedd30aso1163295wmr.5 for ; Mon, 06 Dec 2021 18:46:54 -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=hHIYjo4qBTFosohE3i7TJ7RTmFsSJH6YSE+/8DPvtLY=; b=BW042/rrF1mwlWSw4Ao025r3of6cTFBzwHxeMLlOMCtFiI6oLyf5yeDcjHjuzgT34y TChijgksVG4Z7o6M3IJE0njg8ZgH2eUGlFNuBc6Ux01LB1tzVYvV4lrgj6dEQvg9wR4R pA3OB4YteIe0k3SdZDWvsVAN0A0VZCt/VgoRzxO5i/nAu2M5DTUUqj2K4s7toTuB4EBG M8akw6tQ8bsj8UD3MmveLuIf6oydhPR7sNPAoQNXFGvn5R4PazQzZeMkUFcdac4Vqhh5 HlgVJEYR6C9dVGpkmakMFK5Uaok4AGj5yrU2ikKiUrWASwIiNWDFxfu3a6oo157Vpvj/ gZuw== 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=hHIYjo4qBTFosohE3i7TJ7RTmFsSJH6YSE+/8DPvtLY=; b=bcmCigQ8gDE9v1PZgXVFX9HGFfYO7nR92+c+xHFAQPuVsEx1ZXBKWax082/o05nwJN CBmw73DrVdhP++IUR8vIbianU4+i9mf7gtYwS2IrlEP6kSWkgAT1KQa+QGBWEpJ2wFdH vlrIW4D/Eu+dvKP80y9bCgjPxVGgX0C6GYunagwQaGrMbUn0pm8ik/p8zk3iAsbLAiiB ESE6W1IK8GgsTdRcpx34YO06C/83SMUbMtAEbTOOB6iRE638TEb+TbFxTYu5aQy5t3of +lqB1UgAEkWgVZG/m8GFOJSI6f+mp34Oe+w50+lz/6VZZU9qr09sakFspJWa5u2xdOC6 VIsg== X-Gm-Message-State: AOAM530l+VfpqO1VbORfKnJh05/Db1z0eob7aCDX0LR2kkYPnvb/1YTu vRJivKvC7gxaqv/sTDJ54doHbsfJBTw= X-Google-Smtp-Source: ABdhPJxFw08J1JNdLUfEXeKgJsvlsv3iBAJuZVIRw4Hh99lBsWofGodmvn34dUuyFkvWnmxktAQ+2A== X-Received: by 2002:a1c:208b:: with SMTP id g133mr3319143wmg.128.1638845213155; Mon, 06 Dec 2021 18:46:53 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e7sm16319411wrg.31.2021.12.06.18.46.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Dec 2021 18:46:52 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 07 Dec 2021 02:46:49 +0000 Subject: [PATCH v2 1/3] 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 | 56 +++++++++++++++++++++++++++++ write-or-die.c | 10 +++--- 13 files changed, 165 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..75324c24ee7 --- /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..c3410b8a868 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(_("unknown %s value '%s'"), var, 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..cb9abd7a08c 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..1c5f2c87791 100644 --- a/wrapper.c +++ b/wrapper.c @@ -546,6 +546,62 @@ int xmkstemp_mode(char *filename_template, int mode) return fd; } +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(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. + */ + for (;;) { + int err; +#ifdef __APPLE__ + err = fcntl(fd, F_FULLFSYNC); +#else + err = fsync(fd); +#endif + if (err >= 0 || errno != EINTR) + return err; + } + + 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 Tue Dec 7 02:46:50 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: 12660873 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 6565FC433FE for ; Tue, 7 Dec 2021 02:47:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229500AbhLGCua (ORCPT ); Mon, 6 Dec 2021 21:50:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51098 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230140AbhLGCuZ (ORCPT ); Mon, 6 Dec 2021 21:50:25 -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 DDA85C061746 for ; Mon, 6 Dec 2021 18:46:55 -0800 (PST) Received: by mail-wm1-x333.google.com with SMTP id az34-20020a05600c602200b0033bf8662572so1204680wmb.0 for ; Mon, 06 Dec 2021 18:46:55 -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=rG1ebtc7Pm5O8zZUc/kMunk1WH50KxaS0yXyAxr/u7Y=; b=YS3GY/yFJcCaPB9Yiw2A9UzFk7qOGa/ObrXZcOFBhxavwG1nvMQ7vW0d3clOLreFq9 S9Iqi3esdw9pZmmyDLLgUaRx45BXtXRUXXhf9vdcrN6xLQp/mwhAStaFKRW49rFk/ymb 3U27nFpC6JwFECNl3yinKp+6E+Pqzhv3ayPG+4vFS8eN9RRvh7MXxemkqYENhW7XkHOe pyC5pSGiRsxqtCfJ5W5sBcfGAGycl9Icyjv0vI2wPCRP/5Kz2kl13LdW+fJWLJQAoAXj noUyEgCly2VeN2V243ZMdRRdCT5p6hjnteODO5/h5B2QA2j+ZINQp8/aySY1jRM8BoyQ /ZLg== 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=rG1ebtc7Pm5O8zZUc/kMunk1WH50KxaS0yXyAxr/u7Y=; b=O0pC79a2Jzo70ZY2qAQnN0oJbyGfDQlUsxOcLl44QvB4/tBkrMnkJ4IkpNAzXip28j 1Cm5WPOBoHCdgIC8cISrtV5ya8OqCNLoVwb/M9eS/H+b2zD3QhAhh8uo3Kg9q2Hf5LQg fD+5Po/sq0smGnWTpmgQxdeU2RX6ps1e7UTiasCcFqvuNMXcC+w6C2yNFQerO8jISg8y sOJ6zUVB1iVbT42Bq/Sk8UdQ2d7rqXHkEHEh799k4XopGdrFphdoALQMCNw3EhCvqtGF 4o3E8sp6vSKTb/QqcKnFCMY4eroWfeH5VQLYqOoLjcTSLjOTZEptARQysO5tWmRWgyBs Y77A== X-Gm-Message-State: AOAM531Jk7rQ4XAPtXlABc8Z23lTBrhVZPkVFQ6fULkgB5E73V49ABuB LNGeVKPJzicweIOiAOgCr/eK3HhtHko= X-Google-Smtp-Source: ABdhPJwhj0hWJzlZrej9b/mzq6yKU+MsUqDMEHINgF9yPQQujqiZfKBYFy7T0nUtuFackcO9mp8ZYw== X-Received: by 2002:a1c:a592:: with SMTP id o140mr3275366wme.10.1638845213941; Mon, 06 Dec 2021 18:46:53 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e8sm12830390wrr.26.2021.12.06.18.46.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Dec 2021 18:46:53 -0800 (PST) Message-Id: In-Reply-To: References: Date: Tue, 07 Dec 2021 02:46:50 +0000 Subject: [PATCH v2 2/3] 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 c3410b8a868..29c867aab03 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(_("unknown %s value '%s'"), var, 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 Tue Dec 7 02:46:51 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: 12660871 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 6BFF2C433EF for ; Tue, 7 Dec 2021 02:47:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230204AbhLGCub (ORCPT ); Mon, 6 Dec 2021 21:50:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51100 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230123AbhLGCuZ (ORCPT ); Mon, 6 Dec 2021 21:50:25 -0500 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 22CE3C061354 for ; Mon, 6 Dec 2021 18:46:56 -0800 (PST) Received: by mail-wr1-x42e.google.com with SMTP id a18so26413404wrn.6 for ; Mon, 06 Dec 2021 18:46:56 -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=4560Oo1asFqSCY7CyTBbvQRJTIDfLxJTwwZXXpZWrLw=; b=kyy2lXjzD7sEwSuY3kpu+mSZXAmvSoDI/YfbbIBsb9Xf1fhaMK7XQpLVyrPnI73FM4 nBxaa0wrmPByaagGVqwDOVQbnivTnxXSqGKPzNEnLvKrj6Q+b9OX4m2yOAoVvsf8xmwr hT6C+kfGj40EMcQZZExxk9bsBbRSqP2AF3b2MGlV9XMbtYZzIcOIfjquuYOnia+jG33+ 52pS+GmrhivFlJq/ZiyPCqUvIlKjtAj3WVN4dO3+ZAA/fEzZXeCsPSB7v3cYnEpqvJM+ cMWjXkpI+jKraBeyvM+K1HqxjvYpSEdyXaAu4+3cnwg3OO8rZdFnifShXKH+58FT9YHv dhwA== 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=4560Oo1asFqSCY7CyTBbvQRJTIDfLxJTwwZXXpZWrLw=; b=3pTBjeTuiTT42i1KPwLGmYETfIo4FsHgu2IdArQYmgfb0XhYvnBinErslr5yBf6/Il aCItI0Q76nYHyZEU3S+QqWCwf7KAORpdyILTM9WqJeB+isaFUPsAc4iJG6p592J9uZgM /IXZbvzteoOSqK73aDrs4gGvvn2vcqbYLMiSXtbgK1kIJMbgNTdaRcqLCBjinAQs07j4 BNuvpkpi5ShDbCGHGDzXtXGpU4+wR4pE0axq2VCZKqhFfnsJX/sf48T7zoeEj53wvIU5 uFTCr9Y4ALP7+rMSLn6EWEg/YL14pucjXQDgVKpXugn5jbG3ubtzNPLcig+WXzzduLnl euLA== X-Gm-Message-State: AOAM5329YuBANGVHiLZyl+s91FkE9jihEzuOx0/xE5V0OeYDyNmIQf6X zg2r9BK0teJARYLpBZSc3qQzliClueQ= X-Google-Smtp-Source: ABdhPJy6IiieRg1elPa81HnmXBFOYp18Nw9hct1OpC4DrN0fi+tvqnzQHaZcpaqjIxJjWm62FETNbw== X-Received: by 2002:a5d:6211:: with SMTP id y17mr48308567wru.97.1638845214516; Mon, 06 Dec 2021 18:46:54 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d8sm12859499wrm.76.2021.12.06.18.46.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Dec 2021 18:46:54 -0800 (PST) Message-Id: <86e39b8f8d16100b33346d16e7f722064ab9615c.1638845211.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 07 Dec 2021 02:46:51 +0000 Subject: [PATCH v2 3/3] 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 29c867aab03..17039fa9c10 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))