<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Nov 6, 2017 at 1:31 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Why create a new function?<br>
<br>
Currently rename is only used in FileOutputBuffer.cpp.<br>
<br>
I am OK with renaming rename to allocate_file if you want.<br></blockquote><div><br></div><div>I assume you are talking about resize_file. In the updated patch, I renamed resize_file allocate_file. Now that the original function is gone, I also modified FileOutputBuffer to adopt that change.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
Rafael<br>
<div class="HOEnZb"><div class="h5"><br>
Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> writes:<br>
<br>
> ruiu created this revision.<br>
> Herald added subscribers: hiraditya, mgorny.<br>
><br>
> Unlike fs::resize_file, the new function guarantees that disk blocks<br>
> for a given size are actually allocated on a file system. In other<br>
> words, it doesn't create a sparse file.<br>
><br>
> If you want to do mmap IO, you generally want to preallocate disk<br>
> blocks if you can, because if you mmap a sparse file for writing and<br>
> the disk becomes full, your process will be killed by SIGBUS. There's<br>
> no way to gracefully handle that error condition. If you use fallocate(2)<br>
> or equivalent, you can handle that error before calling mmap.<br>
><br>
> My plan is to use this function in FileOutputBuffer so that we do mmap<br>
> IO only when we can preallocate disk blocks. That prevents LLVM tools<br>
> from crashing with a mysterious bus error when a disk is almost full.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D39464" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39464</a><br>
><br>
> Files:<br>
>   llvm/cmake/config-ix.cmake<br>
>   llvm/include/llvm/Config/<wbr>config.h.cmake<br>
>   llvm/include/llvm/Support/<wbr>FileSystem.h<br>
>   llvm/lib/Support/Unix/Path.inc<br>
>   llvm/lib/Support/Windows/Path.<wbr>inc<br>
><br>
><br>
> Index: llvm/lib/Support/Windows/Path.<wbr>inc<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/lib/Support/Windows/Path.<wbr>inc<br>
> +++ llvm/lib/Support/Windows/Path.<wbr>inc<br>
> @@ -509,6 +509,8 @@<br>
>    return std::error_code(error, std::generic_category());<br>
>  }<br>
><br>
> +std::error_code allocate_file(int FD, uint64_t Size) { resize_file(FD, Size); }<br>
> +<br>
>  std::error_code access(const Twine &Path, AccessMode Mode) {<br>
>    SmallVector<wchar_t, 128> PathUtf16;<br>
><br>
> Index: llvm/lib/Support/Unix/Path.inc<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/lib/Support/Unix/Path.inc<br>
> +++ llvm/lib/Support/Unix/Path.inc<br>
> @@ -438,6 +438,23 @@<br>
>    return std::error_code();<br>
>  }<br>
><br>
> +std::error_code resize_file(int FD, uint64_t Size) {<br>
> +#ifdef HAVE_FALLOCATE<br>
> +  if (fallocate(FD, 0, 0, Size) == -1)<br>
> +    return std::error_code(errno, std::generic_category());<br>
> +  return std::error_code();<br>
> +#endif HAVE_FALLOCATE<br>
> +<br>
> +#ifdef __APPLE__<br>
> +  fstore_t Store = {F_ALLOCATEALL, F_PEOFPOSMODE, 0, Size};<br>
> +  if (fcntl(fd, F_PREALLOCATE, &Store) == -1)<br>
> +    return std::error_code(errno, std::generic_category());<br>
> +  return std::error_code();<br>
> +#endif<br>
> +<br>
> +  return make_error_code(errc::<wbr>function_not_supported);<br>
> +}<br>
> +<br>
>  static int convertAccessMode(AccessMode Mode) {<br>
>    switch (Mode) {<br>
>    case AccessMode::Exist:<br>
> Index: llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> +++ llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> @@ -391,6 +391,14 @@<br>
>  ///          platform-specific error_code.<br>
>  std::error_code resize_file(int FD, uint64_t Size);<br>
><br>
> +/// @brief Preallocate disk blocks to a given file.<br>
> +///<br>
> +/// @param FD Input file descriptor.<br>
> +/// @param Size Size to resize to.<br>
> +/// @returns errc::success if \a path has been resized to \a size, otherwise a<br>
> +///          platform-specific error_code.<br>
> +std::error_code allocate_file(int FD, uint64_t Size);<br>
> +<br>
>  /// @brief Compute an MD5 hash of a file's contents.<br>
>  ///<br>
>  /// @param FD Input file descriptor.<br>
> Index: llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> +++ llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> @@ -59,6 +59,9 @@<br>
>  /* Define to 1 if you have the <errno.h> header file. */<br>
>  #cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}<br>
><br>
> +/* Define to 1 if you have the `fallocate' function. */<br>
> +#cmakedefine HAVE_FALLOCATE ${HAVE_FALLOCATE}<br>
> +<br>
>  /* Define to 1 if you have the <fcntl.h> header file. */<br>
>  #cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}<br>
><br>
> Index: llvm/cmake/config-ix.cmake<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/cmake/config-ix.cmake<br>
> +++ llvm/cmake/config-ix.cmake<br>
> @@ -197,6 +197,11 @@<br>
>  check_symbol_exists(futimens sys/stat.h HAVE_FUTIMENS)<br>
>  check_symbol_exists(futimes sys/time.h HAVE_FUTIMES)<br>
>  check_symbol_exists(posix_<wbr>fallocate fcntl.h HAVE_POSIX_FALLOCATE)<br>
> +<br>
> +set(CMAKE_REQUIRED_<wbr>DEFINITIONS "-D_GNU_SOURCE")<br>
> +check_symbol_exists(fallocate fcntl.h HAVE_FALLOCATE)<br>
> +set(CMAKE_REQUIRED_<wbr>DEFINITIONS "")<br>
> +<br>
>  # AddressSanitizer conflicts with lib/Support/Unix/Signals.inc<br>
>  # Avoid sigaltstack on Apple platforms, where backtrace() cannot handle it<br>
>  # (rdar://7089625) and _Unwind_Backtrace is unusable because it cannot unwind<br>
><br>
><br>
> Index: llvm/lib/Support/Windows/Path.<wbr>inc<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/lib/Support/Windows/Path.<wbr>inc<br>
> +++ llvm/lib/Support/Windows/Path.<wbr>inc<br>
> @@ -509,6 +509,8 @@<br>
>    return std::error_code(error, std::generic_category());<br>
>  }<br>
><br>
> +std::error_code allocate_file(int FD, uint64_t Size) { resize_file(FD, Size); }<br>
> +<br>
>  std::error_code access(const Twine &Path, AccessMode Mode) {<br>
>    SmallVector<wchar_t, 128> PathUtf16;<br>
><br>
> Index: llvm/lib/Support/Unix/Path.inc<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/lib/Support/Unix/Path.inc<br>
> +++ llvm/lib/Support/Unix/Path.inc<br>
> @@ -438,6 +438,23 @@<br>
>    return std::error_code();<br>
>  }<br>
><br>
> +std::error_code resize_file(int FD, uint64_t Size) {<br>
> +#ifdef HAVE_FALLOCATE<br>
> +  if (fallocate(FD, 0, 0, Size) == -1)<br>
> +    return std::error_code(errno, std::generic_category());<br>
> +  return std::error_code();<br>
> +#endif HAVE_FALLOCATE<br>
> +<br>
> +#ifdef __APPLE__<br>
> +  fstore_t Store = {F_ALLOCATEALL, F_PEOFPOSMODE, 0, Size};<br>
> +  if (fcntl(fd, F_PREALLOCATE, &Store) == -1)<br>
> +    return std::error_code(errno, std::generic_category());<br>
> +  return std::error_code();<br>
> +#endif<br>
> +<br>
> +  return make_error_code(errc::<wbr>function_not_supported);<br>
> +}<br>
> +<br>
>  static int convertAccessMode(AccessMode Mode) {<br>
>    switch (Mode) {<br>
>    case AccessMode::Exist:<br>
> Index: llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> +++ llvm/include/llvm/Support/<wbr>FileSystem.h<br>
> @@ -391,6 +391,14 @@<br>
>  ///          platform-specific error_code.<br>
>  std::error_code resize_file(int FD, uint64_t Size);<br>
><br>
> +/// @brief Preallocate disk blocks to a given file.<br>
> +///<br>
> +/// @param FD Input file descriptor.<br>
> +/// @param Size Size to resize to.<br>
> +/// @returns errc::success if \a path has been resized to \a size, otherwise a<br>
> +///          platform-specific error_code.<br>
> +std::error_code allocate_file(int FD, uint64_t Size);<br>
> +<br>
>  /// @brief Compute an MD5 hash of a file's contents.<br>
>  ///<br>
>  /// @param FD Input file descriptor.<br>
> Index: llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> +++ llvm/include/llvm/Config/<wbr>config.h.cmake<br>
> @@ -59,6 +59,9 @@<br>
>  /* Define to 1 if you have the <errno.h> header file. */<br>
>  #cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}<br>
><br>
> +/* Define to 1 if you have the `fallocate' function. */<br>
> +#cmakedefine HAVE_FALLOCATE ${HAVE_FALLOCATE}<br>
> +<br>
>  /* Define to 1 if you have the <fcntl.h> header file. */<br>
>  #cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}<br>
><br>
> Index: llvm/cmake/config-ix.cmake<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/cmake/config-ix.cmake<br>
> +++ llvm/cmake/config-ix.cmake<br>
> @@ -197,6 +197,11 @@<br>
>  check_symbol_exists(futimens sys/stat.h HAVE_FUTIMENS)<br>
>  check_symbol_exists(futimes sys/time.h HAVE_FUTIMES)<br>
>  check_symbol_exists(posix_<wbr>fallocate fcntl.h HAVE_POSIX_FALLOCATE)<br>
> +<br>
> +set(CMAKE_REQUIRED_<wbr>DEFINITIONS "-D_GNU_SOURCE")<br>
> +check_symbol_exists(fallocate fcntl.h HAVE_FALLOCATE)<br>
> +set(CMAKE_REQUIRED_<wbr>DEFINITIONS "")<br>
> +<br>
>  # AddressSanitizer conflicts with lib/Support/Unix/Signals.inc<br>
>  # Avoid sigaltstack on Apple platforms, where backtrace() cannot handle it<br>
>  # (rdar://7089625) and _Unwind_Backtrace is unusable because it cannot unwind<br>
</div></div></blockquote></div><br></div></div>