diff mbox

libc: add fallocate() and fallocate64()

Message ID 20140908221710.GA8663@nbbrfq.cc.univie.ac.at
State Accepted
Commit 33a12b5540b8abbc4ee0ecb3a51912b3c7868517
Headers show

Commit Message

Bernhard Reutner-Fischer Sept. 8, 2014, 10:17 p.m. UTC
On Sun, Sep 07, 2014 at 03:33:46PM -0400, basile@opensource.dyc.edu wrote:
> From: "Anthony G. Basile" <blueness@gentoo.org>
> 
> We add the Linux-specific function fallocate() which allows the user to
> directly manipulate allocate space for a file.  fallocate() can operate
> in different modes, but the default mode is equivalent to posix_fallocate()
> which is specified in POSIX.1.
> 
> Recent releases of e2fsprogs 1.42.11 and above expect fallocate64() to be
> available.

Looks good. A few remarks:

$ egrep "(ADVANCED_R|LINUX_SPEC|LFS)" .config
UCLIBC_HAS_LFS=y
UCLIBC_LINUX_SPECIFIC=y
UCLIBC_HAS_ADVANCED_REALTIME=y

libc/sysdeps/linux/common/fallocate.c:17:22: warning: no previous prototype for '_fallocate' [-Wmissing-prototypes]
 int attribute_hidden _fallocate(int fd, int mode, __off_t offset, __off_t len)
----------------------^
libc/sysdeps/linux/common/fallocate64.c:22:22: warning: no previous prototype for '_fallocate64' [-Wmissing-prototypes]
 int attribute_hidden _fallocate64(int fd, int mode, __off64_t offset, __off64_t len)
----------------------^

   text    data     bss     dec     hex filename
     86       0       0      86      56 posix_fallocate.os
    102       0       0     102      66 posix_fallocate64.os
    142       0       0     142      8e fallocate.os
    143       0       0     143      8f fallocate64.os

Attached incremental patch:

   text    data     bss     dec     hex filename
     86       0       0      86      56 posix_fallocate.os
    102       0       0     102      66 posix_fallocate64.os
    117       0       0     117      75 fallocate.os
    118       0       0     118      76 fallocate64.os

We should drop the libc_hidden_proto(fallocate{,64}) since we do not have
internal users (and iff we gain some they should use __libc_fallocate{,64})
though.
I did not test 32bit vs 64bit targets nor all combos of the egrep above.

Care to test and send an updated patch, please?

TIA,

Comments

Bernhard Reutner-Fischer Sept. 9, 2014, 11:52 a.m. UTC | #1
On 9 September 2014 00:17, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On Sun, Sep 07, 2014 at 03:33:46PM -0400, basile@opensource.dyc.edu wrote:
>> From: "Anthony G. Basile" <blueness@gentoo.org>
>>
>> We add the Linux-specific function fallocate() which allows the user to
>> directly manipulate allocate space for a file.  fallocate() can operate
>> in different modes, but the default mode is equivalent to posix_fallocate()
>> which is specified in POSIX.1.
>>
>> Recent releases of e2fsprogs 1.42.11 and above expect fallocate64() to be
>> available.
>
> Looks good. A few remarks:

> We should drop the libc_hidden_proto(fallocate{,64}) since we do not have
> internal users (and iff we gain some they should use __libc_fallocate{,64})
> though.

I did that and installed your patch with my amendments.

> I did not test 32bit vs 64bit targets nor all combos of the egrep above.
>
> Care to test and send an updated patch, please?

Thanks for the patch and cheers,
Anthony Basile Sept. 10, 2014, 11:35 p.m. UTC | #2
On 09/09/14 07:52, Bernhard Reutner-Fischer wrote:
> On 9 September 2014 00:17, Bernhard Reutner-Fischer
> <rep.dot.nop@gmail.com> wrote:
>> On Sun, Sep 07, 2014 at 03:33:46PM -0400, basile@opensource.dyc.edu wrote:
>>> From: "Anthony G. Basile" <blueness@gentoo.org>
>>>
>>> We add the Linux-specific function fallocate() which allows the user to
>>> directly manipulate allocate space for a file.  fallocate() can operate
>>> in different modes, but the default mode is equivalent to posix_fallocate()
>>> which is specified in POSIX.1.
>>>
>>> Recent releases of e2fsprogs 1.42.11 and above expect fallocate64() to be
>>> available.
>>
>> Looks good. A few remarks:
>
>> We should drop the libc_hidden_proto(fallocate{,64}) since we do not have
>> internal users (and iff we gain some they should use __libc_fallocate{,64})
>> though.
>
> I did that and installed your patch with my amendments.
>
>> I did not test 32bit vs 64bit targets nor all combos of the egrep above.
>>
>> Care to test and send an updated patch, please?
>
> Thanks for the patch and cheers,
>

Hi Bernhard,

I tested the patch on x86_64 and x86, and all is good but one issue. 
When running the tests with

UCLIBC_HAS_LFS=y
UCLIBC_LINUX_SPECIFIC=y
# UCLIBC_HAS_ADVANCED_REALTIME is not set

you hit

tst-posix_fallocate64.o: In function `do_test':
tst-posix_fallocate64.c:(.text+0x62): undefined reference to 
`posix_fallocate'

because the Makefile.in doesn't turn off the build for 
tst-posix_fallocate64, only tst-posix_fallocate.  The logic there is:

ifeq ($(UCLIBC_HAS_LFS),)
TESTS_DISABLED := tst-preadwrite64 tst-posix_fallocate64 tst-fallocate64
endif

ifeq ($(UCLIBC_HAS_ADVANCED_REALTIME),)
TESTS_DISABLED += tst-posix_fallocate
endif

Since LFS is on, tst-posix_fallocate64 is built even though 
UCLIBC_HAS_ADVANCED_REALTIME is not set.  That's why in my original 
patch I changed this to

ifeq ($(UCLIBC_HAS_ADVANCED_REALTIME),)
TESTS_DISABLED += tst-posix_fallocate
ifeq ($(UCLIBC_HAS_LFS),y)
TESTS_DISABLED += tst-posix_fallocate64
endif
endif


Other than that, everything worked and the code reads nicely.  Thanks 
for cleaning things up.

One final point, the patch doesn't apply cleanly to 0.9.33 branch.  Its 
just a couple of simple fixes.  Should I submit a backport patch?  We 
need this in Gentoo to keep up with e2fsprogs.  Mike adds patches from 
the 0.9.33 branch to his patchsets when he bumps the ebuilds.
diff mbox

Patch

diff --git a/include/fcntl.h b/include/fcntl.h
index eb72ce4..a312ca1 100644
--- a/include/fcntl.h
+++ b/include/fcntl.h
@@ -245,8 +245,9 @@  extern int posix_fallocate64 (int __fd, __off64_t __offset, __off64_t __len);
    Unlike the latter, fallocate can operate in different modes.  The default
    mode = 0 is equivalent to posix_fallocate().
 
-   Note: These declarations are used in posix_fallocate.c and posix_fallocate64.c,
-   so we expose them internally.  */
+   Note: These declarations are used in posix_fallocate.c and
+   posix_fallocate64.c, so we expose them internally.
+ */
 
 /* Flags for fallocate's mode.  */
 # define FALLOC_FL_KEEP_SIZE            1 /* Don't extend size of file
@@ -256,6 +257,7 @@  extern int posix_fallocate64 (int __fd, __off64_t __offset, __off64_t __len);
 
 # ifndef __USE_FILE_OFFSET64
 extern int fallocate (int __fd, int __mode, __off_t __offset, __off_t __len);
+libc_hidden_proto(fallocate)
 # else
 #  ifdef __REDIRECT
 extern int __REDIRECT (fallocate, (int __fd, int __mode, __off64_t __offset,
@@ -267,6 +269,7 @@  extern int __REDIRECT (fallocate, (int __fd, int __mode, __off64_t __offset,
 # endif
 # ifdef __USE_LARGEFILE64
 extern int fallocate64 (int __fd, int __mode, __off64_t __offset, __off64_t __len);
+libc_hidden_proto(fallocate64)
 # endif
 #endif
 
diff --git a/libc/sysdeps/linux/common/Makefile.in b/libc/sysdeps/linux/common/Makefile.in
index 98a7af2..8562154 100644
--- a/libc/sysdeps/linux/common/Makefile.in
+++ b/libc/sysdeps/linux/common/Makefile.in
@@ -62,17 +62,10 @@  CSRC-$(UCLIBC_LINUX_SPECIFIC) += \
 	vmsplice.c
 CSRC-$(if $(findstring yy,$(UCLIBC_LINUX_SPECIFIC)$(UCLIBC_HAS_LFS)),y) += \
 	sendfile64.c
-# posix_fallocate() needs the internal function _fallocate() defined in fallocate.c
-# and posix_fallocate64() needs _fallocate64() in fallocate64.c if LFS is enabled.
-# So we do not remove them even if we want just posix_fallocate() or posix_fallocate64().
-ifeq ($(findstring y,$(UCLIBC_LINUX_SPECIFIC)$(UCLIBC_HAS_ADVANCED_REALTIME)),y)
-ifneq ($(findstring y,$(UCLIBC_HAS_LFS)),y)
-CSRC- += fallocate64.c
-endif
-else
-CSRC- += fallocate.c
-CSRC- += fallocate64.c
-endif
+# posix_fallocate() needs __libc_fallocate() from fallocate.c
+# posix_fallocate64() needs __libc_fallocate64() from fallocate64.c
+CSRC-$(if $(UCLIBC_LINUX_SPECIFIC)$(UCLIBC_HAS_ADVANCED_REALTIME),y,) += \
+	fallocate.c $(filter fallocate64.c,$(CSRC-y))
 # NPTL needs these internally: madvise.c
 CSRC-$(findstring y,$(UCLIBC_LINUX_SPECIFIC)$(UCLIBC_HAS_THREADS_NATIVE)) += madvise.c
 ifeq ($(UCLIBC_HAS_THREADS_NATIVE),y)
diff --git a/libc/sysdeps/linux/common/fallocate.c b/libc/sysdeps/linux/common/fallocate.c
index c16cd17..3a43f93 100644
--- a/libc/sysdeps/linux/common/fallocate.c
+++ b/libc/sysdeps/linux/common/fallocate.c
@@ -14,7 +14,8 @@ 
 #include <stdint.h>
 
 #if defined __NR_fallocate
-int attribute_hidden _fallocate(int fd, int mode, __off_t offset, __off_t len)
+extern __typeof(fallocate) __libc_fallocate attribute_hidden;
+int attribute_hidden __libc_fallocate(int fd, int mode, __off_t offset, __off_t len)
 {
 	int ret;
 
@@ -39,13 +40,11 @@  int attribute_hidden _fallocate(int fd, int mode, __off_t offset, __off_t len)
 }
 
 # if defined __UCLIBC_LINUX_SPECIFIC__ && defined __USE_GNU
-int fallocate(int fd, int mode, __off_t offset, __off_t len)
-{
-	return _fallocate(fd, mode, offset, len);
-}
-#  if defined __UCLIBC_HAS_LFS__ && __WORDSIZE == 64
-strong_alias(fallocate,fallocate64)
+strong_alias(__libc_fallocate,fallocate)
+libc_hidden_def(fallocate)
+#  if __WORDSIZE == 64
+strong_alias(__libc_fallocate,fallocate64)
+libc_hidden_def(fallocate64)
 #  endif
 # endif
-
 #endif
diff --git a/libc/sysdeps/linux/common/fallocate64.c b/libc/sysdeps/linux/common/fallocate64.c
index e81c69a..8bacc79 100644
--- a/libc/sysdeps/linux/common/fallocate64.c
+++ b/libc/sysdeps/linux/common/fallocate64.c
@@ -19,7 +19,9 @@ 
 # if __WORDSIZE == 64
 /* Can use normal fallocate() */
 # elif __WORDSIZE == 32
-int attribute_hidden _fallocate64(int fd, int mode, __off64_t offset, __off64_t len)
+extern __typeof(fallocate64) __libc_fallocate64 attribute_hidden;
+int attribute_hidden __libc_fallocate64(int fd, int mode, __off64_t offset,
+		__off64_t len)
 {
 	int ret;
 	INTERNAL_SYSCALL_DECL(err);
@@ -31,10 +33,8 @@  int attribute_hidden _fallocate64(int fd, int mode, __off64_t offset, __off64_t
 }
 
 #  if defined __UCLIBC_LINUX_SPECIFIC__ && defined __USE_GNU
-int fallocate64(int fd, int mode, __off64_t offset, __off64_t len)
-{
-	return _fallocate64(fd, mode, offset, len);
-}
+strong_alias(__libc_fallocate64,fallocate64)
+libc_hidden_def(fallocate64)
 #  endif
 
 # else
diff --git a/libc/sysdeps/linux/common/posix_fallocate.c b/libc/sysdeps/linux/common/posix_fallocate.c
index 9e9b09c..76771e3 100644
--- a/libc/sysdeps/linux/common/posix_fallocate.c
+++ b/libc/sysdeps/linux/common/posix_fallocate.c
@@ -13,12 +13,11 @@ 
 #include <bits/kernel-features.h>
 #include <stdint.h>
 
-extern __typeof(fallocate) _fallocate attribute_hidden;
-
 #if defined __NR_fallocate
+extern __typeof(fallocate) __libc_fallocate attribute_hidden;
 int posix_fallocate(int fd, __off_t offset, __off_t len)
 {
-	return _fallocate(fd, 0, offset, len);
+	return __libc_fallocate(fd, 0, offset, len);
 }
 # if defined __UCLIBC_HAS_LFS__ && __WORDSIZE == 64
 strong_alias(posix_fallocate,posix_fallocate64)
diff --git a/libc/sysdeps/linux/common/posix_fallocate64.c b/libc/sysdeps/linux/common/posix_fallocate64.c
index d5875ca..12ddbc2 100644
--- a/libc/sysdeps/linux/common/posix_fallocate64.c
+++ b/libc/sysdeps/linux/common/posix_fallocate64.c
@@ -13,17 +13,16 @@ 
 #include <bits/kernel-features.h>
 #include <stdint.h>
 
-extern __typeof(fallocate64) _fallocate64 attribute_hidden;
-
 #if defined __NR_fallocate
 # if __WORDSIZE == 64
 /* Can use normal posix_fallocate() */
 # elif __WORDSIZE == 32
+extern __typeof(fallocate64) __libc_fallocate64 attribute_hidden;
 int posix_fallocate64(int fd, __off64_t offset, __off64_t len)
 {
-	return _fallocate64(fd, 0, offset, len);
+	return __libc_fallocate64(fd, 0, offset, len);
 }
 # else
-# error your machine is neither 32 bit or 64 bit ... it must be magical
+#  error your machine is neither 32 bit or 64 bit ... it must be magical
 # endif
 #endif
diff --git a/test/unistd/Makefile.in b/test/unistd/Makefile.in
index 0aaefb5..ed33d9a 100644
--- a/test/unistd/Makefile.in
+++ b/test/unistd/Makefile.in
@@ -6,21 +6,15 @@  ifeq ($(UCLIBC_HAS_LFS),)
 TESTS_DISABLED := tst-preadwrite64 tst-posix_fallocate64 tst-fallocate64
 endif
 
-# If we don't have LINUX_SPECIFIC, then get rid of tst-fallocate, and check to
-# make sure we didn't miss removing tst-fallocate64 above because LFS is enabled
+# If we don't have LINUX_SPECIFIC, then get rid of tst-fallocate
 ifeq ($(UCLIBC_LINUX_SPECIFIC),)
 TESTS_DISABLED += tst-fallocate
-ifeq ($(UCLIBC_HAS_LFS),y)
-TESTS_DISABLED += tst-fallocate64
-endif
 endif
 
-# The logic is similar for HAS_ADVANCED_REALTIME and tst-posix_fallocate/tst-posix_fallocate64
+# The logic is similar for HAS_ADVANCED_REALTIME and
+# tst-posix_fallocate/tst-posix_fallocate64
 ifeq ($(UCLIBC_HAS_ADVANCED_REALTIME),)
 TESTS_DISABLED += tst-posix_fallocate
-ifeq ($(UCLIBC_HAS_LFS),y)
-TESTS_DISABLED += tst-posix_fallocate64
-endif
 endif
 
 OPTS_getopt      := -abcXXX -9
diff --git a/test/unistd/tst-fallocate.c b/test/unistd/tst-fallocate.c
index 2b15596..0f61821 100644
--- a/test/unistd/tst-fallocate.c
+++ b/test/unistd/tst-fallocate.c
@@ -10,12 +10,6 @@ 
 # endif
 #endif
 
-static void do_prepare (void);
-#define PREPARE(argc, argv) do_prepare ()
-static int do_test (void);
-#define TEST_FUNCTION do_test ()
-#include <test-skeleton.c>
-
 static int fd;
 static void
 do_prepare (void)
@@ -161,3 +155,7 @@  do_test (void)
 
   return 0;
 }
+
+#define PREPARE(argc, argv) do_prepare ()
+#define TEST_FUNCTION do_test ()
+#include <test-skeleton.c>