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

[Michał Górny via Phabricator via llvm-commits]

mgorny added inline comments.


================
Comment at: llvm/lib/Support/Unix/Path.inc:443
+#ifdef HAVE_FALLOCATE
+  if (fallocate(FD, 0, 0, Size) == -1)
+    return std::error_code(errno, std::generic_category());
----------------
ruiu wrote:
> mgorny wrote:
> > ruiu wrote:
> > > mgorny wrote:
> > > > ruiu wrote:
> > > > > kettenis wrote:
> > > > > > mgorny wrote:
> > > > > > > ruiu wrote:
> > > > > > > > mgorny wrote:
> > > > > > > > > Any reason not to use `posix_fallocate()` which is more portable by definition?
> > > > > > > > `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.
> > > > > > > Ok then.
> > > > > > 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).
> > > > > 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.
> > > > 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?
> > > 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.
> > 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?
> > 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.
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.


https://reviews.llvm.org/D39464