Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File preallocationSize: align Windows and Unix behavior. #59338

Merged
merged 18 commits into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ internal static partial class Interop
internal static partial class Sys
{
/// <summary>
/// Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned.
/// Returns -1 on error, 0 on success.
/// </summary>
[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PosixFAllocate", SetLastError = false)]
internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length);
[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FAllocate", SetLastError = true)]
internal static extern int FAllocate(SafeFileHandle fd, long offset, long length);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

internal static partial class Interop
{
internal static partial class Kernel32
{
// Value taken from https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle#remarks:
internal const int FileAllocationInfo = 5;

internal struct FILE_ALLOCATION_INFO
{
internal long AllocationSize;
}
}
}
tmds marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 1 addition & 2 deletions src/libraries/Native/Unix/Common/pal_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
#cmakedefine01 HAVE_STRLCAT
#cmakedefine01 HAVE_SHM_OPEN_THAT_WORKS_WELL_ENOUGH_WITH_MMAP
#cmakedefine01 HAVE_POSIX_ADVISE
#cmakedefine01 HAVE_POSIX_FALLOCATE
#cmakedefine01 HAVE_POSIX_FALLOCATE64
#cmakedefine01 HAVE_FALLOCATE
#cmakedefine01 HAVE_PREADV
#cmakedefine01 HAVE_PWRITEV
#cmakedefine01 PRIORITY_REQUIRES_INT_WHO
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Native/Unix/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ static const Entry s_sysNative[] =
DllImportEntry(SystemNative_FTruncate)
DllImportEntry(SystemNative_Poll)
DllImportEntry(SystemNative_PosixFAdvise)
DllImportEntry(SystemNative_PosixFAllocate)
DllImportEntry(SystemNative_FAllocate)
DllImportEntry(SystemNative_Read)
DllImportEntry(SystemNative_ReadLink)
DllImportEntry(SystemNative_Rename)
Expand Down
74 changes: 11 additions & 63 deletions src/libraries/Native/Unix/System.Native/pal_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -1008,83 +1008,31 @@ int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t length, i
#endif
}

int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length)
int32_t SystemNative_FAllocate(intptr_t fd, int64_t offset, int64_t length)
{
assert_msg(offset == 0, "Invalid offset value", (int)offset);

int fileDescriptor = ToFileDescriptor(fd);
int32_t result;
#if HAVE_POSIX_FALLOCATE64 // 64-bit Linux
while ((result = posix_fallocate64(fileDescriptor, (off64_t)offset, (off64_t)length)) == EINTR);
#elif HAVE_POSIX_FALLOCATE // 32-bit Linux
tmds marked this conversation as resolved.
Show resolved Hide resolved
while ((result = posix_fallocate(fileDescriptor, (off_t)offset, (off_t)length)) == EINTR);
#if HAVE_FALLOCATE // Linux
while ((result = fallocate(fileDescriptor, FALLOC_FL_KEEP_SIZE, (off_t)offset, (off_t)length)) == EINTR);
tmds marked this conversation as resolved.
Show resolved Hide resolved
#elif defined(F_PREALLOCATE) // macOS
fstore_t fstore;
fstore.fst_flags = F_ALLOCATECONTIG; // ensure contiguous space
fstore.fst_posmode = F_PEOFPOSMODE; // allocate from the physical end of file, as offset MUST NOT be 0 for F_VOLPOSMODE
fstore.fst_flags = F_ALLOCATEALL; // Allocate all requested space or no space at all.
fstore.fst_posmode = 0;
tmds marked this conversation as resolved.
Show resolved Hide resolved
fstore.fst_offset = (off_t)offset;
fstore.fst_length = (off_t)length;
fstore.fst_bytesalloc = 0; // output size, can be > length

while ((result = fcntl(fileDescriptor, F_PREALLOCATE, &fstore)) == -1 && errno == EINTR);

if (result == -1)
{
// we have failed to allocate contiguous space, let's try non-contiguous
fstore.fst_flags = F_ALLOCATEALL; // all or nothing
while ((result = fcntl(fileDescriptor, F_PREALLOCATE, &fstore)) == -1 && errno == EINTR);
}
#elif defined(F_ALLOCSP) || defined(F_ALLOCSP64) // FreeBSD
#if HAVE_FLOCK64
struct flock64 lockArgs;
int command = F_ALLOCSP64;
#else
struct flock lockArgs;
int command = F_ALLOCSP;
#endif

lockArgs.l_whence = SEEK_SET;
lockArgs.l_start = (off_t)offset;
lockArgs.l_len = (off_t)length;

while ((result = fcntl(fileDescriptor, command, &lockArgs)) == -1 && errno == EINTR);
#endif

#if defined(F_PREALLOCATE) || defined(F_ALLOCSP) || defined(F_ALLOCSP64)
// most of the Unixes implement posix_fallocate which does NOT set the last error
// fctnl does, but to mimic the posix_fallocate behaviour we just return error
if (result == -1)
{
result = errno;
}
else
{
// align the behaviour with what posix_fallocate does (change reported file size)
ftruncate(fileDescriptor, length);
}
#else
(void)offset; // unused
(void)length; // unused
result = -1;
errno = EOPNOTSUPP;
#endif

// error codes can be OS-specific, so this is why this handling is done here rather than in the managed layer
switch (result)
{
case ENOSPC: // there was not enough space
return -1;
case EFBIG: // the file was too large
return -2;
case ENODEV: // not a regular file
case ESPIPE: // a pipe
// We ignore it, as FileStream contract makes it clear that allocationSize is ignored for non-regular files.
return 0;
case EINVAL:
// We control the offset and length so they are correct.
assert_msg(length >= 0, "Invalid length value", (int)length);
// But if the underlying filesystem does not support the operation, we just ignore it and treat as a hint.
return 0;
default:
assert(result != EINTR); // it can't happen here as we retry the call on EINTR
assert(result != EBADF); // it can't happen here as this method is being called after a succesfull call to open (with write permissions) before returning the SafeFileHandle to the user
return 0;
}
return result;
}

int32_t SystemNative_Read(intptr_t fd, void* buffer, int32_t bufferSize)
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/Native/Unix/System.Native/pal_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -620,11 +620,11 @@ PALEXPORT int32_t SystemNative_Poll(PollEvent* pollEvents, uint32_t eventCount,
PALEXPORT int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t length, int32_t advice);

/**
* Ensures that disk space is allocated.
* Preallocates disk space.
*
* Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned.
* Returns 0 for success, -1 for failure. Sets errno on failure.
*/
PALEXPORT int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length);
PALEXPORT int32_t SystemNative_FAllocate(intptr_t fd, int64_t offset, int64_t length);

/**
* Reads the number of bytes specified into the provided buffer from the specified, opened file descriptor.
Expand Down
9 changes: 2 additions & 7 deletions src/libraries/Native/Unix/configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,9 @@ check_symbol_exists(
HAVE_POSIX_ADVISE)

check_symbol_exists(
posix_fallocate
fallocate
fcntl.h
HAVE_POSIX_FALLOCATE)

check_symbol_exists(
posix_fallocate64
fcntl.h
HAVE_POSIX_FALLOCATE64)
HAVE_FALLOCATE)

check_symbol_exists(
preadv
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.IO.FileSystem/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@
<data name="Argument_InvalidAppendMode" xml:space="preserve">
<value>Append access can be requested only in write-only mode.</value>
</data>
<data name="Argument_InvalidPreallocateAccess" xml:space="preserve">
<value>preallocation can be requested only in write mode.</value>
tmds marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="Argument_InvalidPreallocateMode" xml:space="preserve">
<value>preallocation can be requested only for new or truncated files.</value>
tmds marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="Argument_InvalidFileModeAndAccessCombo" xml:space="preserve">
<value>Combining FileMode: {0} with FileAccess: {1} is invalid.</value>
</data>
Expand Down
9 changes: 3 additions & 6 deletions src/libraries/System.IO.FileSystem/tests/File/Open.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode)
return File.Open(path,
new FileStreamOptions {
Mode = mode,
Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite,
PreallocationSize = PreallocationSize
Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite
});
}

Expand All @@ -59,8 +58,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
return File.Open(path,
new FileStreamOptions {
Mode = mode,
Access = access,
PreallocationSize = PreallocationSize
Access = access
});
}

Expand All @@ -72,8 +70,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
Access = access,
Share = share,
Options = options,
BufferSize = bufferSize,
PreallocationSize = PreallocationSize
BufferSize = bufferSize
tmds marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ public class File_OpenHandle : FileStream_ctor_options_as
protected override FileStream CreateFileStream(string path, FileMode mode)
{
FileAccess access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite;
return new FileStream(File.OpenHandle(path, mode, access, preallocationSize: PreallocationSize), access);
return new FileStream(File.OpenHandle(path, mode, access), access);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the verdict on testing coverage with all of these tests being updated to not pass in a preallocation size?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage is good now.

I refactored tests to follow the pattern that was already used by adding an overload of CreateFileStream to the base class FileStream_ctor_options that accepts a preallocationSize and add the preallocationSize related tests in this class. The derived classes use it from different public APIs.

}

protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access)
=> new FileStream(File.OpenHandle(path, mode, access, preallocationSize: PreallocationSize), access);
=> new FileStream(File.OpenHandle(path, mode, access), access);

protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
=> new FileStream(File.OpenHandle(path, mode, access, share, options, PreallocationSize), access, bufferSize, (options & FileOptions.Asynchronous) != 0);
=> new FileStream(File.OpenHandle(path, mode, access, share, options), access, bufferSize, (options & FileOptions.Asynchronous) != 0);

[Fact]
public override void NegativePreallocationSizeThrows()
Expand Down
9 changes: 3 additions & 6 deletions src/libraries/System.IO.FileSystem/tests/FileInfo/Open.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode)
return new FileInfo(path).Open(
new FileStreamOptions {
Mode = mode,
Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite,
PreallocationSize = PreallocationSize
Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite
});
}

Expand All @@ -91,8 +90,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
return new FileInfo(path).Open(
new FileStreamOptions {
Mode = mode,
Access = access,
PreallocationSize = PreallocationSize
Access = access
});
}

Expand All @@ -104,8 +102,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
Access = access,
Share = share,
Options = options,
BufferSize = bufferSize,
PreallocationSize = PreallocationSize
BufferSize = bufferSize
});
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.IO.Tests
{
public partial class FileStream_ctor_options_as
{
private static long GetAllocatedSize(FileStream fileStream)
{
return 0;
}

private static bool SupportsPreallocation => false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.InteropServices;

namespace System.IO.Tests
{
public partial class FileStream_ctor_options_as
{
private static long GetAllocatedSize(FileStream fileStream)
{
// Call 'stat' to get the number of blocks, and size of blocks.
using var px = Process.Start(new ProcessStartInfo
{
FileName = "stat",
ArgumentList = { "-c", "%b %B", fileStream.Name },
RedirectStandardOutput = true
});
string stdout = px.StandardOutput.ReadToEnd();

string[] parts = stdout.Split(' ');
return long.Parse(parts[0]) * long.Parse(parts[1]);
}

private static bool SupportsPreallocation =>
RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ||
RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ namespace System.IO.Tests
{
public partial class FileStream_ctor_options_as
{
private unsafe long GetAllocatedSize(FileStream fileStream)
{
Interop.Kernel32.FILE_STANDARD_INFO info;

Assert.True(Interop.Kernel32.GetFileInformationByHandleEx(fileStream.SafeFileHandle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO)));

return info.AllocationSize;
}

[Theory]
[InlineData(@"\\?\")]
[InlineData(@"\??\")]
Expand All @@ -23,31 +32,15 @@ public void ExtendedPathsAreSupported(string prefix)

using (var fs = new FileStream(filePath, GetOptions(FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize)))
{
Assert.Equal(preallocationSize, fs.Length);
}
}

[Fact]
public async Task PreallocationSizeIsIgnoredForNonSeekableFiles()
{
string pipeName = GetNamedPipeServerStreamName();
string pipePath = Path.GetFullPath($@"\\.\pipe\{pipeName}");

FileStreamOptions options = new() { Mode = FileMode.Open, Access = FileAccess.Write, Share = FileShare.None, PreallocationSize = 123 };

using (var server = new NamedPipeServerStream(pipeName, PipeDirection.In))
using (var clienStream = new FileStream(pipePath, options))
{
await server.WaitForConnectionAsync();

Assert.False(clienStream.CanSeek);
Assert.Equal(0, fs.Length);
Assert.Equal(preallocationSize, GetAllocatedSize(fs));
}
}

[ConditionalTheory(nameof(IsFat32))]
[InlineData(FileMode.Create)]
[InlineData(FileMode.CreateNew)]
[InlineData(FileMode.OpenOrCreate)]
[InlineData(FileMode.Truncate)]
public void WhenFileIsTooLargeTheErrorMessageContainsAllDetails(FileMode mode)
{
const long tooMuch = uint.MaxValue + 1L; // more than FAT32 max size
Expand Down Expand Up @@ -99,5 +92,7 @@ public extern static bool GetVolumeInformation(
out uint fileSystemFlags,
StringBuilder fileSystemNameBuffer,
int fileSystemNameSize);

private static bool SupportsPreallocation => true;
}
}
Loading