[PATCH] D39464: Define fs::allocate_file which preallocates disk blocks.

[Rui Ueyama via llvm-commits]

On Mon, Nov 6, 2017 at 1:31 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Why create a new function?
>
> Currently rename is only used in FileOutputBuffer.cpp.
>
> I am OK with renaming rename to allocate_file if you want.
>

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.

Cheers,
> Rafael
>
> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>
> > ruiu created this revision.
> > Herald added subscribers: hiraditya, mgorny.
> >
> > Unlike fs::resize_file, the new function guarantees that disk blocks
> > for a given size are actually allocated on a file system. In other
> > words, it doesn't create a sparse file.
> >
> > If you want to do mmap IO, you generally want to preallocate disk
> > blocks if you can, because if you mmap a sparse file for writing and
> > the disk becomes full, your process will be killed by SIGBUS. There's
> > no way to gracefully handle that error condition. If you use fallocate(2)
> > or equivalent, you can handle that error before calling mmap.
> >
> > My plan is to use this function in FileOutputBuffer so that we do mmap
> > IO only when we can preallocate disk blocks. That prevents LLVM tools
> > from crashing with a mysterious bus error when a disk is almost full.
> >
> >
> > https://reviews.llvm.org/D39464
> >
> > Files:
> >   llvm/cmake/config-ix.cmake
> >   llvm/include/llvm/Config/config.h.cmake
> >   llvm/include/llvm/Support/FileSystem.h
> >   llvm/lib/Support/Unix/Path.inc
> >   llvm/lib/Support/Windows/Path.inc
> >
> >
> > Index: llvm/lib/Support/Windows/Path.inc
> > ===================================================================
> > --- llvm/lib/Support/Windows/Path.inc
> > +++ llvm/lib/Support/Windows/Path.inc
> > @@ -509,6 +509,8 @@
> >    return std::error_code(error, std::generic_category());
> >  }
> >
> > +std::error_code allocate_file(int FD, uint64_t Size) { resize_file(FD,
> Size); }
> > +
> >  std::error_code access(const Twine &Path, AccessMode Mode) {
> >    SmallVector<wchar_t, 128> PathUtf16;
> >
> > Index: llvm/lib/Support/Unix/Path.inc
> > ===================================================================
> > --- llvm/lib/Support/Unix/Path.inc
> > +++ llvm/lib/Support/Unix/Path.inc
> > @@ -438,6 +438,23 @@
> >    return std::error_code();
> >  }
> >
> > +std::error_code resize_file(int FD, uint64_t Size) {
> > +#ifdef HAVE_FALLOCATE
> > +  if (fallocate(FD, 0, 0, Size) == -1)
> > +    return std::error_code(errno, std::generic_category());
> > +  return std::error_code();
> > +#endif HAVE_FALLOCATE
> > +
> > +#ifdef __APPLE__
> > +  fstore_t Store = {F_ALLOCATEALL, F_PEOFPOSMODE, 0, Size};
> > +  if (fcntl(fd, F_PREALLOCATE, &Store) == -1)
> > +    return std::error_code(errno, std::generic_category());
> > +  return std::error_code();
> > +#endif
> > +
> > +  return make_error_code(errc::function_not_supported);
> > +}
> > +
> >  static int convertAccessMode(AccessMode Mode) {
> >    switch (Mode) {
> >    case AccessMode::Exist:
> > Index: llvm/include/llvm/Support/FileSystem.h
> > ===================================================================
> > --- llvm/include/llvm/Support/FileSystem.h
> > +++ llvm/include/llvm/Support/FileSystem.h
> > @@ -391,6 +391,14 @@
> >  ///          platform-specific error_code.
> >  std::error_code resize_file(int FD, uint64_t Size);
> >
> > +/// @brief Preallocate disk blocks to a given file.
> > +///
> > +/// @param FD Input file descriptor.
> > +/// @param Size Size to resize to.
> > +/// @returns errc::success if \a path has been resized to \a size,
> otherwise a
> > +///          platform-specific error_code.
> > +std::error_code allocate_file(int FD, uint64_t Size);
> > +
> >  /// @brief Compute an MD5 hash of a file's contents.
> >  ///
> >  /// @param FD Input file descriptor.
> > Index: llvm/include/llvm/Config/config.h.cmake
> > ===================================================================
> > --- llvm/include/llvm/Config/config.h.cmake
> > +++ llvm/include/llvm/Config/config.h.cmake
> > @@ -59,6 +59,9 @@
> >  /* Define to 1 if you have the <errno.h> header file. */
> >  #cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}
> >
> > +/* Define to 1 if you have the `fallocate' function. */
> > +#cmakedefine HAVE_FALLOCATE ${HAVE_FALLOCATE}
> > +
> >  /* Define to 1 if you have the <fcntl.h> header file. */
> >  #cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}
> >
> > Index: llvm/cmake/config-ix.cmake
> > ===================================================================
> > --- llvm/cmake/config-ix.cmake
> > +++ llvm/cmake/config-ix.cmake
> > @@ -197,6 +197,11 @@
> >  check_symbol_exists(futimens sys/stat.h HAVE_FUTIMENS)
> >  check_symbol_exists(futimes sys/time.h HAVE_FUTIMES)
> >  check_symbol_exists(posix_fallocate fcntl.h HAVE_POSIX_FALLOCATE)
> > +
> > +set(CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
> > +check_symbol_exists(fallocate fcntl.h HAVE_FALLOCATE)
> > +set(CMAKE_REQUIRED_DEFINITIONS "")
> > +
> >  # AddressSanitizer conflicts with lib/Support/Unix/Signals.inc
> >  # Avoid sigaltstack on Apple platforms, where backtrace() cannot handle
> it
> >  # (rdar://7089625) and _Unwind_Backtrace is unusable because it cannot
> unwind
> >
> >
> > Index: llvm/lib/Support/Windows/Path.inc
> > ===================================================================
> > --- llvm/lib/Support/Windows/Path.inc
> > +++ llvm/lib/Support/Windows/Path.inc
> > @@ -509,6 +509,8 @@
> >    return std::error_code(error, std::generic_category());
> >  }
> >
> > +std::error_code allocate_file(int FD, uint64_t Size) { resize_file(FD,
> Size); }
> > +
> >  std::error_code access(const Twine &Path, AccessMode Mode) {
> >    SmallVector<wchar_t, 128> PathUtf16;
> >
> > Index: llvm/lib/Support/Unix/Path.inc
> > ===================================================================
> > --- llvm/lib/Support/Unix/Path.inc
> > +++ llvm/lib/Support/Unix/Path.inc
> > @@ -438,6 +438,23 @@
> >    return std::error_code();
> >  }
> >
> > +std::error_code resize_file(int FD, uint64_t Size) {
> > +#ifdef HAVE_FALLOCATE
> > +  if (fallocate(FD, 0, 0, Size) == -1)
> > +    return std::error_code(errno, std::generic_category());
> > +  return std::error_code();
> > +#endif HAVE_FALLOCATE
> > +
> > +#ifdef __APPLE__
> > +  fstore_t Store = {F_ALLOCATEALL, F_PEOFPOSMODE, 0, Size};
> > +  if (fcntl(fd, F_PREALLOCATE, &Store) == -1)
> > +    return std::error_code(errno, std::generic_category());
> > +  return std::error_code();
> > +#endif
> > +
> > +  return make_error_code(errc::function_not_supported);
> > +}
> > +
> >  static int convertAccessMode(AccessMode Mode) {
> >    switch (Mode) {
> >    case AccessMode::Exist:
> > Index: llvm/include/llvm/Support/FileSystem.h
> > ===================================================================
> > --- llvm/include/llvm/Support/FileSystem.h
> > +++ llvm/include/llvm/Support/FileSystem.h
> > @@ -391,6 +391,14 @@
> >  ///          platform-specific error_code.
> >  std::error_code resize_file(int FD, uint64_t Size);
> >
> > +/// @brief Preallocate disk blocks to a given file.
> > +///
> > +/// @param FD Input file descriptor.
> > +/// @param Size Size to resize to.
> > +/// @returns errc::success if \a path has been resized to \a size,
> otherwise a
> > +///          platform-specific error_code.
> > +std::error_code allocate_file(int FD, uint64_t Size);
> > +
> >  /// @brief Compute an MD5 hash of a file's contents.
> >  ///
> >  /// @param FD Input file descriptor.
> > Index: llvm/include/llvm/Config/config.h.cmake
> > ===================================================================
> > --- llvm/include/llvm/Config/config.h.cmake
> > +++ llvm/include/llvm/Config/config.h.cmake
> > @@ -59,6 +59,9 @@
> >  /* Define to 1 if you have the <errno.h> header file. */
> >  #cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}
> >
> > +/* Define to 1 if you have the `fallocate' function. */
> > +#cmakedefine HAVE_FALLOCATE ${HAVE_FALLOCATE}
> > +
> >  /* Define to 1 if you have the <fcntl.h> header file. */
> >  #cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}
> >
> > Index: llvm/cmake/config-ix.cmake
> > ===================================================================
> > --- llvm/cmake/config-ix.cmake
> > +++ llvm/cmake/config-ix.cmake
> > @@ -197,6 +197,11 @@
> >  check_symbol_exists(futimens sys/stat.h HAVE_FUTIMENS)
> >  check_symbol_exists(futimes sys/time.h HAVE_FUTIMES)
> >  check_symbol_exists(posix_fallocate fcntl.h HAVE_POSIX_FALLOCATE)
> > +
> > +set(CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
> > +check_symbol_exists(fallocate fcntl.h HAVE_FALLOCATE)
> > +set(CMAKE_REQUIRED_DEFINITIONS "")
> > +
> >  # AddressSanitizer conflicts with lib/Support/Unix/Signals.inc
> >  # Avoid sigaltstack on Apple platforms, where backtrace() cannot handle
> it
> >  # (rdar://7089625) and _Unwind_Backtrace is unusable because it cannot
> unwind
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171108/8b55c836/attachment.html>