Define fs::allocate_file which preallocates disk blocks.

[ruiu]

Summary

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.

[ruiu]

• Fix typo

[mgorny]

llvm/lib/Support/Unix/Path.inc

453

Any reason not to use posix_fallocate() which is more portable by definition?

[ruiu]

llvm/lib/Support/Unix/Path.inc

453

posix_fallocate does not fail even on filesystems that do not support block preallocation. What it does (at least in glibc) when the underlying filesystem doesn't have the feature is to write a zero byte for each disk block to force the filesystem to actually allocate disk blocks. It is as you can imagine pretty slow.

[mgorny]

llvm/lib/Support/Unix/Path.inc

453

Ok then.

[kettenis]

llvm/lib/Support/Unix/Path.inc

453

Can I ask you to reconsider this? fallocate(2) is Linux-specific and other systems are likely to only implement the standardized posix_fallocate(2). And while the posix_fallocate(2) implementation might be somewhat suboptimal on filesystems that don't implement fallocate(2), this should only affect the small number of users that use lld on non-standard filesystems.

When I said that we might consider implementing fallocate(2) on OpenBSD, I really meant to say that we will consider implementing posix_fallocate(2).

[ruiu]

llvm/lib/Support/Unix/Path.inc

453

I don't know if we really want to hide fallocate and posix_fallocate because the performance characteristics of the two functions are so different when they are used on a file system that doesn't support block preallocation. I believe posix_fallocate shouldn't have completely abstracted away that difference.

That said I understand your concern too. This code might be too specific to Linux, and it just does nothing for other Unix systems. That's bad.

How about this: we can make this function to call posix_fallocate and make FileOutputBuffer to use this function only when the disk is almost full. Then in most cases we don't need to call posix_fallocate at all on any system, and it can still catch disk full errors in practice.

[mgorny]

llvm/lib/Support/Unix/Path.inc

453

I'd dare say relying on 'free space' to be meaningful and stable is a very bad idea.

I'm going to hate myself for suggesting this but how about using fallocate() specifically on Linux, and posix_fallocate() otherwise?

[ruiu]

llvm/lib/Support/Unix/Path.inc

453

Why do you think that is so bad? In the worse case scenario, we can't get a correct value of disk free space and lld is killed by sigbus due to a disk full error. But that's the same as the current situation. (I'm not suggesting we return a disk full error if a disk looks like almost full. What I'm suggesting is to call posix_fallocate only when a disk looks almost full.)

I'm going to hate myself for suggesting this but how about using fallocate() specifically on Linux, and posix_fallocate() otherwise?

I'm reluctant to do that because it feels like we are treating non-Linux Unix systems as second class citizens -- we know that posix_fallocate is slow on many Unixes, so calling that function makes lld slower on many non-Linux systems.

[mgorny]

llvm/lib/Support/Unix/Path.inc

453

But AFAIU avoiding SIGBUS is the purpose of this whole effort — and with modern filesystems (I'm thinking btrfs) 'free space' became something very inaccurate to rely on. I think it's better to have a safe behavior that avoids SIGBUS altogether.

I'm reluctant to do that because it feels like we are treating non-Linux Unix systems as second class citizens -- we know that posix_fallocate is slow on many Unixes, so calling that function makes lld slower on many non-Linux systems.

Then just make it not use mmap when it can't ensure a safe & fast allocation?

[ruiu]

llvm/lib/Support/Unix/Path.inc

453

But AFAIU avoiding SIGBUS is the purpose of this whole effort — and with modern filesystems (I'm thinking btrfs) 'free space' became something very inaccurate to rely on. I think it's better to have a safe behavior that avoids SIGBUS altogether.

How much inaccurate thy are? If you are using a filesystem nearly full, that is slow anyway, so I'm not worry too much about calling posix_fallocate conservatively. Say, we can call posix_fallocate() when disk look like 90% full. is this still unacceptably inaccurate?

Then just make it not use mmap when it can't ensure a safe & fast allocation?

That's slower than mmap. Basically, I want to use mmap, but the issue of the sparse file & mmap write came up, so I'm trying to find a way to detect an error without sacrificing performance too much.

[mgorny]

llvm/lib/Support/Unix/Path.inc

453

If you're talking about the data from statvfs(), then you shouldn't trust its value at all. The data straight from btrfs tools might be better but with all the high magic involved in it, it can fail to fail to write with any apparent free space.

Then, there's the matter of quotas.

[emaste]

llvm/lib/Support/Unix/Path.inc

453

posix_fallocate does not fail even on filesystems that do not support block preallocation.

It can, in 1003.1-2008: [EINVAL] The len argument is less than zero, or the offset argument is less than zero, or the underlying file system does not support this operation.

[ruiu]

• Rename resize_file allocate_file
• Make FileOutputBuffer to use allocate_file instead of resize_file

[ruiu]

• Fix tests

[bcain]

I think it's a mistake to have the behavior be contingent on available disk space for the destination partition. Has that proposal been ruled out now?

llvm/lib/Support/Unix/Path.inc

443

Does this code compile with __APPLE__? It looks like this typo means it won't compile. s/fd/FD/

[krytarowski]

llvm/lib/Support/Unix/Path.inc

436

NetBSD needs ftruncate(2) as a fallback for posix_fallocate(2).

In the default setup posix_fallocate() returns EOPNOTSUPP.

[ruiu]

llvm/lib/Support/Unix/Path.inc

436

The current code use ftruncate if the function is available. So it doesn't use it as a fallback, but it uses only fallocate. It seems like a OK behavior to me on NetBSD if posix_fallocate always returns EOPNOTSUPP, but can you confirm?

[krytarowski]

llvm/lib/Support/Unix/Path.inc

436

By "the current code" I understand the pristine one. It calls posix_fallocate(2) and if it fails with EOPNOTSUPP, it moves on to the ftruncate(2) fallback.

As far as I can tell, this fallocate is a missing feature as of today in the FFSv2 file-system, the default one on NetBSD.

[ruiu]

llvm/lib/Support/Unix/Path.inc

436

Ah, sorry, I misread ftruncate fallocate.

I intentionally do not fallback to ftruncate in this function. This function's contract is to allocate actual disk blocks to a given file or return with a failure code. On the caller side, if this function returns an error, we use an in-memory buffer instead of an on-disk temporary file.

[joerg]

I really dislike this direction. fallocate can double the amount of disk IO and increase cache trashing, especially when linking large programs with debug information. Keeping more things in memory doesn't sound like an actual improvement either. If the goal is really only to improve the diagnostics in tools, I think a better idea would be to figure out a good way to handle this from a SIGBUS handler based on the passed in siginfo_t.

llvm/include/llvm/Support/FileSystem.h

 std::error_code rename(const Twine &from, const Twine &to);
 
 /// @brief Copy the contents of \a From to \a To.
 ///
 /// @param From The path to copy from.
 /// @param To The path to copy to. This is created.
 std::error_code copy_file(const Twine &From, const Twine &To);
 
-/// @brief Resize path to size. File is resized as if by POSIX truncate().
+/// @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 resize_file(int FD, uint64_t Size);
+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.
 /// @returns An MD5Result with the hash computed, if successful, otherwise a
 ///          std::error_code.
 ErrorOr<MD5::MD5Result> md5_contents(int FD);
 

llvm/lib/Support/FileOutputBuffer.cpp

createInMemoryBuffer(StringRef Path, size_t Size, unsigned Mode) {
   std::error_code EC;
   MemoryBlock MB = Memory::allocateMappedMemory(
       Size, nullptr, sys::Memory::MF_READ | sys::Memory::MF_WRITE, EC);
   if (EC)
     return errorCodeToError(EC);
   return llvm::make_unique<InMemoryBuffer>(Path, MB, Mode);
 }
 
-static Expected<std::unique_ptr<OnDiskBuffer>>
+static Expected<std::unique_ptr<FileOutputBuffer>>
 createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
   // Create new file in same directory but with random name.
   SmallString<128> TempPath;
   int FD;
   if (auto EC = fs::createUniqueFile(Path + ".tmp%%%%%%%", FD, TempPath, Mode))
     return errorCodeToError(EC);
 
   sys::RemoveFileOnSignal(TempPath);
 
 #ifndef LLVM_ON_WIN32
-  // On Windows, CreateFileMapping (the mmap function on Windows)
-  // automatically extends the underlying file. We don't need to
-  // extend the file beforehand. _chsize (ftruncate on Windows) is
-  // pretty slow just like it writes specified amount of bytes,
-  // so we should avoid calling that function.
-  if (auto EC = fs::resize_file(FD, Size))
+  // A signal (usually a SIGBUS) is raised when a disk becomes full while
+  // writing to a sparse file using mmap. There's no portable nor reliable
+  // way to handle such error condition. In order to prevent that error, we
+  // extend a file and then preallocate disk blocks now.
+  //
+  // Not all operating systems nor filesystems support block preallocation.
+  // If it is not supported, we use an in-memory buffer instead so that the
+  // disk full error can be caught on commit().
+  //
+  // On Windows, we don't need to do this because CreateFileMapping (the
+  // mmap function on Windows) automatically extends an undeflying file.
+  // Pre-extending a file is just a waste of time.
+  if (auto EC = fs::allocate_file(FD, Size)) {
+    fs::remove(TempPath);
+    if (EC == errc::function_not_supported)
+      return createInMemoryBuffer(Path, Size, Mode);
     return errorCodeToError(EC);
+  }
 #endif
 
   // Mmap it.
   std::error_code EC;
   auto MappedFile = llvm::make_unique<fs::mapped_file_region>(
       FD, fs::mapped_file_region::readwrite, Size, 0, EC);
   close(FD);
   if (EC)

llvm/lib/Support/Unix/Path.inc

std::error_code rename(const Twine &from, const Twine &to) {
   StringRef t = to.toNullTerminatedStringRef(to_storage);
 
   if (::rename(f.begin(), t.begin()) == -1)
     return std::error_code(errno, std::generic_category());
 
   return std::error_code();
 }
 
-std::error_code resize_file(int FD, uint64_t Size) {
-#if defined(HAVE_POSIX_FALLOCATE)
-  // If we have posix_fallocate use it. Unlike ftruncate it always allocates
-  // space, so we get an error if the disk is full.
+// Preallocate disk blocks.
+std::error_code allocate_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
+
+#ifdef HAVE_POSIX_FALLOCATE
   if (int Err = ::posix_fallocate(FD, 0, Size)) {
     if (Err != EINVAL && Err != EOPNOTSUPP)
       return std::error_code(Err, std::generic_category());
+    return make_error_code(errc::function_not_supported);
   }
+  return std::error_code();
 #endif
-  // Use ftruncate as a fallback. It may or may not allocate space. At least on
-  // OS X with HFS+ it does.
-  if (::ftruncate(FD, Size) == -1)
-    return std::error_code(errno, std::generic_category());
 
+#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:
     return F_OK;
   case AccessMode::Write:
     return W_OK;
   case AccessMode::Execute:
     return R_OK | X_OK; // scripts also need R_OK.
   }
   llvm_unreachable("invalid enum");
 }

llvm/lib/Support/Windows/Path.inc

for (unsigned Retry = 0; Retry != 200; ++Retry) {
     // process might have raced with us to create and open the destination
     // file, so we need to keep doing this until we succeed.
   }
 
   // The most likely root cause.
   return errc::permission_denied;
 }
 
-std::error_code resize_file(int FD, uint64_t Size) {
+std::error_code allocate_file(int FD, uint64_t Size) {
 #ifdef HAVE__CHSIZE_S
   errno_t error = ::_chsize_s(FD, Size);
 #else
   errno_t error = ::_chsize(FD, Size);
 #endif
   return std::error_code(error, std::generic_category());
 }
 

llvm/unittests/Support/Path.cpp

path::append(FilePathname, "test");
     auto Buf = MemoryBuffer::getFile(FilePathname.str());
     EXPECT_TRUE((bool)Buf);
     EXPECT_EQ(Buf.get()->getBuffer(), "\n");
   }
   ASSERT_NO_ERROR(fs::remove(Twine(FilePathname)));
 }
 #endif
 
-TEST_F(FileSystemTest, Resize) {
+TEST_F(FileSystemTest, Allocate) {
   int FD;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "temp", FD, TempPath));
-  ASSERT_NO_ERROR(fs::resize_file(FD, 123));
+  std::error_code EC = fs::allocate_file(FD, 123);
+  if (!EC) {
     fs::file_status Status;
     ASSERT_NO_ERROR(fs::status(FD, Status));
     ASSERT_EQ(Status.getSize(), 123U);
+  }
   ::close(FD);
   ASSERT_NO_ERROR(fs::remove(TempPath));
 }
 
 TEST_F(FileSystemTest, MD5) {
   int FD;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "temp", FD, TempPath));

 
 TEST_F(FileSystemTest, FileMapping) {
   // Create a temp file.
   int FileDescriptor;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
   unsigned Size = 4096;
-  ASSERT_NO_ERROR(fs::resize_file(FileDescriptor, Size));
+  if (fs::allocate_file(FileDescriptor, Size)) {
+    ASSERT_NO_ERROR(fs::remove(TempPath));
+    return;
+  }
 
   // Map in temp file and add some content
   std::error_code EC;
   StringRef Val("hello there");
   {
     fs::mapped_file_region mfr(FileDescriptor,
                                fs::mapped_file_region::readwrite, Size, 0, EC);
     ASSERT_NO_ERROR(EC);