[PATCH v3 1/2] ntdll: Use posix_fallocate(2/3) instead of fallocate(2).
From: Charles Davis <cdavis5x(a)gmail.com> The fallocate(2) call only exists on Linux. posix_fallocate() is more portable. Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- v3: Fix typo. v2: Don't fall back to fallocate(2). --- configure.ac | 10 +--------- dlls/ntdll/unix/file.c | 9 +++++---- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index fec39b6f2ac..cc8a4b79aaf 100644 --- a/configure.ac +++ b/configure.ac @@ -2183,6 +2183,7 @@ AC_CHECK_FUNCS(\ poll \ port_create \ posix_fadvise \ + posix_fallocate \ prctl \ proc_pidinfo \ readlink \ @@ -2274,15 +2275,6 @@ then AC_DEFINE(HAVE_SCHED_SETAFFINITY, 1, [Define to 1 if you have the `sched_setaffinity' function.]) fi -AC_CACHE_CHECK([for fallocate],wine_cv_have_fallocate, - AC_LINK_IFELSE([AC_LANG_PROGRAM( -[[#define _GNU_SOURCE -#include <fcntl.h>]], [[fallocate(-1, 0, 0, 0);]])],[wine_cv_have_fallocate=yes],[wine_cv_have_fallocate=no])) -if test "$wine_cv_have_fallocate" = "yes" -then - AC_DEFINE(HAVE_FALLOCATE, 1, [Define to 1 if you have the `fallocate' function.]) -fi - dnl **** Check for types **** AC_C_INLINE diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 046338b2a82..c92cbd1db4a 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4574,11 +4574,12 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, status = STATUS_INVALID_PARAMETER; else { -#ifdef HAVE_FALLOCATE - if (fallocate( fd, 0, 0, (off_t)info->ValidDataLength.QuadPart ) == -1) +#ifdef HAVE_POSIX_FALLOCATE + int err; + if ((err = posix_fallocate( fd, 0, (off_t)info->ValidDataLength.QuadPart )) != 0) { - if (errno == EOPNOTSUPP) WARN( "fallocate not supported on this filesystem\n" ); - else status = errno_to_status( errno ); + if (err == EOPNOTSUPP) WARN( "posix_fallocate not supported on this filesystem\n" ); + else status = errno_to_status( err ); } #else FIXME( "setting valid data length not supported\n" ); -- 2.33.0
From: Charles Davis <cdavis5x(a)gmail.com> Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index c92cbd1db4a..899cdc9868a 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4581,6 +4581,22 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (err == EOPNOTSUPP) WARN( "posix_fallocate not supported on this filesystem\n" ); else status = errno_to_status( err ); } +#elif defined(F_PREALLOCATE) + struct stat st; + + if (fstat( fd, &st ) < 0) + status = errno_to_status( errno ); + else + { + struct fstore fst; + + fst.fst_flags = F_ALLOCATECONTIG|F_ALLOCATEALL; + fst.fst_posmode = F_PEOFPOSMODE; + fst.fst_offset = 0; + fst.fst_length = (off_t)info->ValidDataLength.QuadPart - st.st_blocks * 512; + if (fcntl( fd, F_PREALLOCATE, &fst ) < 0) + status = errno_to_status( errno ); + } #else FIXME( "setting valid data length not supported\n" ); #endif -- 2.33.0
On Tue, Sep 14, 2021 at 04:57:42PM -0500, Chip Davis wrote:
From: Charles Davis <cdavis5x(a)gmail.com>
Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index c92cbd1db4a..899cdc9868a 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4581,6 +4581,22 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (err == EOPNOTSUPP) WARN( "posix_fallocate not supported on this filesystem\n" ); else status = errno_to_status( err ); } +#elif defined(F_PREALLOCATE) + struct stat st; + + if (fstat( fd, &st ) < 0) + status = errno_to_status( errno );
There's already a call to fstat() in the outer block so this seems unnecessary.
+ else + { + struct fstore fst; + + fst.fst_flags = F_ALLOCATECONTIG|F_ALLOCATEALL; + fst.fst_posmode = F_PEOFPOSMODE; + fst.fst_offset = 0; + fst.fst_length = (off_t)info->ValidDataLength.QuadPart - st.st_blocks * 512;
Ignoring the potential for a race here, shouldn't you fetch the block size from fstatfs() rather than hard-coding it?
+ if (fcntl( fd, F_PREALLOCATE, &fst ) < 0) + status = errno_to_status( errno ); + }
Huw.
Huw Davies <huw(a)codeweavers.com> writes:
On Tue, Sep 14, 2021 at 04:57:42PM -0500, Chip Davis wrote:
From: Charles Davis <cdavis5x(a)gmail.com>
Signed-off-by: Chip Davis <cdavis(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index c92cbd1db4a..899cdc9868a 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4581,6 +4581,22 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (err == EOPNOTSUPP) WARN( "posix_fallocate not supported on this filesystem\n" ); else status = errno_to_status( err ); } +#elif defined(F_PREALLOCATE) + struct stat st; + + if (fstat( fd, &st ) < 0) + status = errno_to_status( errno );
There's already a call to fstat() in the outer block so this seems unnecessary.
+ else + { + struct fstore fst; + + fst.fst_flags = F_ALLOCATECONTIG|F_ALLOCATEALL; + fst.fst_posmode = F_PEOFPOSMODE; + fst.fst_offset = 0; + fst.fst_length = (off_t)info->ValidDataLength.QuadPart - st.st_blocks * 512;
Ignoring the potential for a race here, shouldn't you fetch the block size from fstatfs() rather than hard-coding it?
+ if (fcntl( fd, F_PREALLOCATE, &fst ) < 0) + status = errno_to_status( errno ); + }
Do we actually need this in the first place? I thought posix_fallocate() was supposed to be the portable solution <g> -- Alexandre Julliard julliard(a)winehq.org
On Wed, Sep 15, 2021 at 10:52:35AM +0200, Alexandre Julliard wrote:
Do we actually need this in the first place? I thought posix_fallocate() was supposed to be the portable solution <g>
It's not available on macOS (at least not on 10.14), so apparently it's not that portable ;-/ Huw.
Huw Davies <huw(a)codeweavers.com> writes:
On Wed, Sep 15, 2021 at 10:52:35AM +0200, Alexandre Julliard wrote:
Do we actually need this in the first place? I thought posix_fallocate() was supposed to be the portable solution <g>
It's not available on macOS (at least not on 10.14), so apparently it's not that portable ;-/
I'd suggest to ignore it then. It's only an optimization, and F_PREALLOCATE doesn't seem like a good match for this. -- Alexandre Julliard julliard(a)winehq.org
September 15, 2021 4:39 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
Huw Davies <huw(a)codeweavers.com> writes:
On Wed, Sep 15, 2021 at 10:52:35AM +0200, Alexandre Julliard wrote:
Do we actually need this in the first place? I thought posix_fallocate() was supposed to be the portable solution <g>
It's not available on macOS (at least not on 10.14), so apparently it's not that portable ;-/
I'd suggest to ignore it then. It's only an optimization, and F_PREALLOCATE doesn't seem like a good match for this.
Honestly, I only wrote this one to shut up the FIXME that gets emitted every time Steam installs something. Maybe demoting it to a WARN is more apropos, then. Now if you'll excuse me, I'm off to beg Apple once again for a new feature... Chip
participants (3)
-
Alexandre Julliard -
Chip Davis -
Huw Davies