git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/3] core.fsyncmethod: add writeout-only mode

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 7, 2021, 11:44 UTC
Message-ID
<Ya9JJlItvDJCLHqj@ncase>
In-Reply-To
<e79522cbdd4feb45b062b75225475f34039d1866.1638845211.git.gitgitgadget@gmail.com>
On Tue, Dec 07, 2021 at 02:46:49AM +0000, Neeraj Singh via GitGitGadget wrote:
> From: Neeraj Singh <neerajsi@microsoft.com>
[snip]
Show 36 quoted lines
> --- 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 <winternl.h>
> +#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;
> +       }

I'm wondering whether it would make sense to fall back to fsync(3P) in case we cannot use writeout-only, but I see that were doing essentially that in `fsync_or_die()`. There is no indicator to the user though that writeout-only doesn't work -- do we want to print a one-time warning?

Show 9 quoted lines
> +       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;
> +}
[snip]
Show 19 quoted lines
> 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);

Below we're looping around `EINTR` -- are Apple systems never returning it?

Patrick
Show 72 quoted lines
> +#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)
> -- 
> gitgitgadget
> 
Previous: Neeraj Singh via GitGitGadgetNext: Ævar Arnfjörð Bjarmason
Message 7 of 122 in “A design for future-proofing fsync() configuration”
  1. 0/2 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Dec 4, 2021
  2. 1/2 fsync: add writeout-only mode for fsyncing repo dataNeeraj Singh via GitGitGadget, Dec 4, 2021
  3. Neeraj SinghDec 6, 2021
  4. 2/2 core.fsync: introduce granular fsync controlNeeraj Singh via GitGitGadget, Dec 4, 2021
  5. 0/3 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Dec 7, 2021
  6. 1/3 core.fsyncmethod: add writeout-only modeNeeraj Singh via GitGitGadget, Dec 7, 2021
  7. Patrick SteinhardtDec 7, 2021
  8. Ævar Arnfjörð BjarmasonDec 7, 2021
  9. Neeraj SinghDec 7, 2021
  10. Ævar Arnfjörð BjarmasonDec 7, 2021
  11. Neeraj SinghDec 7, 2021
  12. 2/3 core.fsync: introduce granular fsync controlNeeraj Singh via GitGitGadget, Dec 7, 2021
  13. Patrick SteinhardtDec 7, 2021
  14. Neeraj SinghDec 7, 2021
  15. Ævar Arnfjörð BjarmasonDec 7, 2021
  16. Neeraj SinghDec 7, 2021
  17. Ævar Arnfjörð BjarmasonDec 8, 2021
  18. Neeraj SinghDec 9, 2021
  19. Junio C HamanoDec 9, 2021
  20. Ævar Arnfjörð BjarmasonDec 9, 2021
  21. Neeraj SinghDec 9, 2021
  22. Neeraj SinghJan 18, 2022
  23. Ævar Arnfjörð BjarmasonJan 19, 2022
  24. Ævar Arnfjörð BjarmasonJan 19, 2022
  25. Neeraj SinghJan 28, 2022
  26. 3/3 core.fsync: new option to harden the indexNeeraj Singh via GitGitGadget, Dec 7, 2021
  27. Patrick SteinhardtDec 7, 2021
  28. Neeraj SinghDec 8, 2021
  29. 0/4 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Dec 9, 2021
  30. 1/4 core.fsyncmethod: add writeout-only modeNeeraj Singh via GitGitGadget, Dec 9, 2021
  31. 2/4 core.fsync: introduce granular fsync controlNeeraj Singh via GitGitGadget, Dec 9, 2021
  32. 3/4 core.fsync: new option to harden the indexNeeraj Singh via GitGitGadget, Dec 9, 2021
  33. 4/4 core.fsync: add a `derived-metadata` aggregate optionNeeraj Singh via GitGitGadget, Dec 9, 2021
  34. Neeraj SinghJan 8, 2022
  35. rsbecker@nexbridge.comJan 9, 2022
  36. Neeraj SinghJan 10, 2022
  37. 0/4 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Feb 1, 2022
  38. 1/4 core.fsyncmethod: add writeout-only modeNeeraj Singh via GitGitGadget, Feb 1, 2022
  39. 2/4 core.fsync: introduce granular fsync controlNeeraj Singh via GitGitGadget, Feb 1, 2022
  40. Junio C HamanoFeb 2, 2022
  41. Junio C HamanoFeb 2, 2022
  42. Neeraj SinghFeb 11, 2022
  43. Junio C HamanoFeb 11, 2022
  44. Neeraj SinghFeb 11, 2022
  45. Junio C HamanoFeb 11, 2022
  46. rsbecker@nexbridge.comFeb 12, 2022
  47. Patrick SteinhardtFeb 14, 2022
  48. Junio C HamanoFeb 14, 2022
  49. Patrick SteinhardtMar 9, 2022
  50. Ævar Arnfjörð BjarmasonMar 9, 2022
  51. Junio C HamanoMar 9, 2022
  52. Patrick SteinhardtMar 10, 2022
  53. Junio C HamanoMar 10, 2022
  54. Neeraj SinghMar 9, 2022
  55. Neeraj SinghFeb 11, 2022
  56. 3/4 core.fsync: new option to harden the indexNeeraj Singh via GitGitGadget, Feb 1, 2022
  57. 4/4 core.fsync: add a `derived-metadata` aggregate optionNeeraj Singh via GitGitGadget, Feb 1, 2022
  58. 0/5 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Mar 9, 2022
  59. 1/5 wrapper: move inclusion of CSPRNG headers the wrapper.c fileNeeraj Singh via GitGitGadget, Mar 9, 2022
  60. Junio C HamanoMar 9, 2022
  61. Neeraj SinghMar 10, 2022
  62. brian m. carlsonMar 10, 2022
  63. Neeraj SinghMar 10, 2022
  64. 2/5 core.fsyncmethod: add writeout-only modeNeeraj Singh via GitGitGadget, Mar 9, 2022
  65. Junio C HamanoMar 9, 2022
  66. 3/5 core.fsync: introduce granular fsync controlNeeraj Singh via GitGitGadget, Mar 9, 2022
  67. Junio C HamanoMar 10, 2022
  68. Neeraj SinghMar 10, 2022
  69. Junio C HamanoMar 10, 2022
  70. Neeraj SinghMar 10, 2022
  71. Junio C HamanoMar 10, 2022
  72. Junio C HamanoMar 10, 2022
  73. Neeraj SinghMar 10, 2022
  74. Junio C HamanoMar 10, 2022
  75. Johannes SchindelinMar 10, 2022
  76. Junio C HamanoMar 10, 2022
  77. 4/5 core.fsync: new option to harden the indexNeeraj Singh via GitGitGadget, Mar 9, 2022
  78. 5/5 core.fsync: documentation and user-friendly aggregate optionsNeeraj Singh via GitGitGadget, Mar 9, 2022
  79. Future-proofed syncing of refsPatrick Steinhardt, Mar 10, 2022
  80. 6/8 core.fsync: add `fsync_component()` wrapper which doesn't diePatrick Steinhardt, Mar 10, 2022
  81. Junio C HamanoMar 10, 2022
  82. Neeraj SinghMar 10, 2022
  83. 7/8 core.fsync: new option to harden loose referencesPatrick Steinhardt, Mar 10, 2022
  84. Junio C HamanoMar 10, 2022
  85. Neeraj SinghMar 10, 2022
  86. Neeraj SinghMar 10, 2022
  87. Junio C HamanoMar 11, 2022
  88. Patrick SteinhardtMar 11, 2022
  89. Ævar Arnfjörð BjarmasonMar 11, 2022
  90. 8/8 core.fsync: new option to harden packed referencesPatrick Steinhardt, Mar 10, 2022
  91. Junio C HamanoMar 10, 2022
  92. Patrick SteinhardtMar 11, 2022
  93. 0/6 A design for future-proofing fsync() configurationNeeraj K. Singh via GitGitGadget, Mar 10, 2022
  94. 1/6 wrapper: make inclusion of Windows csprng header tightly scopedNeeraj Singh via GitGitGadget, Mar 10, 2022
  95. 3/6 core.fsync: introduce granular fsync control infrastructureNeeraj Singh via GitGitGadget, Mar 10, 2022
  96. 4/6 core.fsync: add configuration parsingNeeraj Singh via GitGitGadget, Mar 10, 2022
  97. Jiang XinMar 28, 2022
  98. Neeraj SinghMar 28, 2022
  99. 5/6 core.fsync: new option to harden the indexNeeraj Singh via GitGitGadget, Mar 10, 2022
  100. 2/6 core.fsyncmethod: add writeout-only modeNeeraj Singh via GitGitGadget, Mar 10, 2022
  101. 6/6 core.fsync: documentation and user-friendly aggregate optionsNeeraj Singh via GitGitGadget, Mar 10, 2022
  102. core.fsync: documentation and user-friendly aggregate optionsNeeraj Singh, Mar 15, 2022
  103. Junio C HamanoMar 15, 2022
  104. Neeraj SinghMar 15, 2022
  105. do we have too much fsync() configuration in 'next'? (was: [PATCH v7] core.fsync: documentation and user-friendly aggregate options)Ævar Arnfjörð Bjarmason, Mar 23, 2022
  106. Neeraj SinghMar 25, 2022
  107. Ævar Arnfjörð BjarmasonMar 26, 2022
  108. Junio C HamanoMar 26, 2022
  109. Neeraj SinghMar 26, 2022
  110. Ævar Arnfjörð BjarmasonMar 26, 2022
  111. Neeraj SinghMar 27, 2022
  112. Ævar Arnfjörð BjarmasonMar 27, 2022
  113. Patrick SteinhardtMar 28, 2022
  114. Ævar Arnfjörð BjarmasonMar 28, 2022
  115. Neeraj SinghMar 28, 2022
  116. Neeraj SinghMar 30, 2022
  117. Junio C HamanoMar 10, 2022
  118. Neeraj SinghMar 11, 2022
  119. Neeraj SinghMar 11, 2022
  120. Junio C HamanoMar 13, 2022
  121. core.fsync: new option to harden referencesPatrick Steinhardt, Mar 11, 2022
  122. SZEDER GáborMar 25, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.