[1/2] Add nocancel version of pread64()

Message ID MWHPR2101MB0732AF4AE4ECE64CB603997BE8DA0@MWHPR2101MB0732.namprd21.prod.outlook.com
State Committed
Headers

Commit Message

Leandro Pereira Aug. 5, 2019, 9:54 p.m. UTC
  This is in preparation for changes in the dynamic linker so that pread() is used instead of lseek()+read().

2019-08-05  Leandro Pereira  <leandro.pereira@microsoft.com>

	* sysdeps/generic/not-cancel.h: New macro.
	* sysdeps/unix/sysv/linux/not-cancel.h: New macro.
	* sysdeps/unix/sysv/linux/pread64_nocancel.c: Implement nocancel
	version of pread64.
	* sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add
	__pread64_nocancel.
	* sysdeps/unix/sysv/linux/Makefile: Reference pread64_nocancel.c.

---
 sysdeps/generic/not-cancel.h               |  2 ++
 sysdeps/unix/sysv/linux/Makefile           |  4 +--
 sysdeps/unix/sysv/linux/Versions           |  1 +
 sysdeps/unix/sysv/linux/not-cancel.h       |  4 +++
 sysdeps/unix/sysv/linux/pread64_nocancel.c | 32 ++++++++++++++++++++++
 5 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/pread64_nocancel.c
  

Comments

Carlos O'Donell Aug. 6, 2019, 5:09 p.m. UTC | #1
On 8/5/19 5:54 PM, Leandro Pereira wrote:
> This is in preparation for changes in the dynamic linker so that pread() is used instead of lseek()+read().
> 
> 2019-08-05  Leandro Pereira  <leandro.pereira@microsoft.com>
> 
> 	* sysdeps/generic/not-cancel.h: New macro.
> 	* sysdeps/unix/sysv/linux/not-cancel.h: New macro.
> 	* sysdeps/unix/sysv/linux/pread64_nocancel.c: Implement nocancel
> 	version of pread64.
> 	* sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add
> 	__pread64_nocancel.
> 	* sysdeps/unix/sysv/linux/Makefile: Reference pread64_nocancel.c.

I'm am really excited to see changes like this going into glibc.
Whenever we get a chance to do a small optimization like this is great.

I don't see a copyright disclaimer for you on file, and I'm going to
follow up with you privately to see what we can do there to expedite
the process so we can work on your patches as quickly as possible.

Thank you again!
  
Leandro Pereira Sept. 6, 2019, 11:15 p.m. UTC | #2
On August 6th 2019, Carlos O'Donnel wrote:

>> 2019-08-05  Leandro Pereira  <leandro.pereira@microsoft.com>
>>
>>        * sysdeps/generic/not-cancel.h: New macro.
>>        * sysdeps/unix/sysv/linux/not-cancel.h: New macro.
>>        * sysdeps/unix/sysv/linux/pread64_nocancel.c: Implement nocancel
>>        version of pread64.
>>        * sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add
>>        __pread64_nocancel.
>>        * sysdeps/unix/sysv/linux/Makefile: Reference pread64_nocancel.c.
>
> (...)
> I don't see a copyright disclaimer for you on file, and I'm going to
> follow up with you privately to see what we can do there to expedite
> the process so we can work on your patches as quickly as possible.  

With the exception of the boilerplate copied from existing glibc code
(essentially the patch adding the pread() syscall wrapper), the legally
significant changes are only the changes to use pread() instead of
seek()+read(); and those are less than around 15 lines of code.  This should
be fine according to FSFs guidelines.     

Since the changes are small, it falls under an "de minimis" exception rule
for open source contributions at Microsoft, making it a lot easier to
contribute this patch.  If we need to contribute larger changes, then we 
can work out the copyright paperwork.  (Or if I contribute changes as
a private individual, of course.)

        Leandro
  
Carlos O'Donell Oct. 3, 2019, 1:55 p.m. UTC | #3
On 8/5/19 5:54 PM, Leandro Pereira wrote:
> This is in preparation for changes in the dynamic linker so that pread() is used instead of lseek()+read().
> 
> 2019-08-05  Leandro Pereira  <leandro.pereira@microsoft.com>
> 
> 	* sysdeps/generic/not-cancel.h: New macro.
> 	* sysdeps/unix/sysv/linux/not-cancel.h: New macro.
> 	* sysdeps/unix/sysv/linux/pread64_nocancel.c: Implement nocancel
> 	version of pread64.
> 	* sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add
> 	__pread64_nocancel.
> 	* sysdeps/unix/sysv/linux/Makefile: Reference pread64_nocancel.c.

OK for master. I'll push after testing with bmg.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sysdeps/generic/not-cancel.h               |  2 ++
>  sysdeps/unix/sysv/linux/Makefile           |  4 +--
>  sysdeps/unix/sysv/linux/Versions           |  1 +
>  sysdeps/unix/sysv/linux/not-cancel.h       |  4 +++
>  sysdeps/unix/sysv/linux/pread64_nocancel.c | 32 ++++++++++++++++++++++
>  5 files changed, 41 insertions(+), 2 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/pread64_nocancel.c
> 
> diff --git a/sysdeps/generic/not-cancel.h b/sysdeps/generic/not-cancel.h
> index f3d862651e..724c3e5e71 100644
> --- a/sysdeps/generic/not-cancel.h
> +++ b/sysdeps/generic/not-cancel.h
> @@ -41,6 +41,8 @@
>    (void) __close (fd)
>  #define __read_nocancel(fd, buf, n) \
>    __read (fd, buf, n)
> +#define __pread64_nocancel(fd, buf, count, offset) \
> +  __pread64 (fd, buf, count, offset)

OK.

>  #define __write_nocancel(fd, buf, n) \
>    __write (fd, buf, n)
>  #define __writev_nocancel_nostatus(fd, iov, n) \
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 1ab6bcbfc8..a7980a60f6 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -212,8 +212,8 @@ sysdep_routines += xstatconv internal_statvfs internal_statvfs64 \
>  		   close_nocancel fcntl_nocancel nanosleep_nocancel \
>  		   open_nocancel open64_nocancel \
>  		   openat_nocancel openat64_nocancel \
> -		   pause_nocancel read_nocancel waitpid_nocancel \
> -		   write_nocancel statx_cp
> +		   pause_nocancel read_nocancel pread64_nocancel \
> +		   waitpid_nocancel write_nocancel statx_cp

OK.

>  
>  sysdep_headers += bits/fcntl-linux.h
>  
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index 1ca102a9e2..d385085c61 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -182,6 +182,7 @@ libc {
>      __syscall_rt_sigqueueinfo;
>      __open_nocancel;
>      __read_nocancel;
> +    __pread64_nocancel;

OK. Adds GLIBC_PRIVATE interface for loader.

>      __close_nocancel;
>      __sigtimedwait;
>      # functions used by nscd
> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
> index 16cc31cba5..bf7d80125a 100644
> --- a/sysdeps/unix/sysv/linux/not-cancel.h
> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
> @@ -43,6 +43,9 @@ __typeof (openat64) __openat64_nocancel;
>  /* Non cancellable read syscall.  */
>  __typeof (__read) __read_nocancel;
>  
> +/* Non cancellable pread syscall (LFS version).  */
> +__typeof (__pread64) __pread64_nocancel;
> +

OK.

>  /* Uncancelable write.  */
>  __typeof (__write) __write_nocancel;
>  
> @@ -84,6 +87,7 @@ hidden_proto (__open64_nocancel)
>  hidden_proto (__openat_nocancel)
>  hidden_proto (__openat64_nocancel)
>  hidden_proto (__read_nocancel)
> +hidden_proto (__pread64_nocancel)

OK.

>  hidden_proto (__write_nocancel)
>  hidden_proto (__close_nocancel)
>  hidden_proto (__waitpid_nocancel)
> diff --git a/sysdeps/unix/sysv/linux/pread64_nocancel.c b/sysdeps/unix/sysv/linux/pread64_nocancel.c
> new file mode 100644
> index 0000000000..dab61260e5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/pread64_nocancel.c
> @@ -0,0 +1,32 @@
> +/* Linux pread64() syscall implementation -- non-cancellable.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <unistd.h>
> +#include <sysdep-cancel.h>
> +#include <not-cancel.h>
> +
> +#ifndef __NR_pread64
> +# define __NR_pread64 __NR_pread
> +#endif
> +
> +ssize_t
> +__pread64_nocancel (int fd, void *buf, size_t count, off64_t offset)
> +{
> +  return INLINE_SYSCALL_CALL (pread64, fd, buf, count, SYSCALL_LL64_PRW (offset));
> +}
> +hidden_def (__pread64_nocancel)

OK.

>
  
Carlos O'Donell Oct. 18, 2019, 8:02 p.m. UTC | #4
On 10/3/19 9:55 AM, Carlos O'Donell wrote:
> On 8/5/19 5:54 PM, Leandro Pereira wrote:
>> This is in preparation for changes in the dynamic linker so that pread() is used instead of lseek()+read().
>>
>> 2019-08-05  Leandro Pereira  <leandro.pereira@microsoft.com>
>>
>> 	* sysdeps/generic/not-cancel.h: New macro.
>> 	* sysdeps/unix/sysv/linux/not-cancel.h: New macro.
>> 	* sysdeps/unix/sysv/linux/pread64_nocancel.c: Implement nocancel
>> 	version of pread64.
>> 	* sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add
>> 	__pread64_nocancel.
>> 	* sysdeps/unix/sysv/linux/Makefile: Reference pread64_nocancel.c.
> 
> OK for master. I'll push after testing with bmg.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Pushed.
  

Patch

diff --git a/sysdeps/generic/not-cancel.h b/sysdeps/generic/not-cancel.h
index f3d862651e..724c3e5e71 100644
--- a/sysdeps/generic/not-cancel.h
+++ b/sysdeps/generic/not-cancel.h
@@ -41,6 +41,8 @@ 
   (void) __close (fd)
 #define __read_nocancel(fd, buf, n) \
   __read (fd, buf, n)
+#define __pread64_nocancel(fd, buf, count, offset) \
+  __pread64 (fd, buf, count, offset)
 #define __write_nocancel(fd, buf, n) \
   __write (fd, buf, n)
 #define __writev_nocancel_nostatus(fd, iov, n) \
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 1ab6bcbfc8..a7980a60f6 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -212,8 +212,8 @@  sysdep_routines += xstatconv internal_statvfs internal_statvfs64 \
 		   close_nocancel fcntl_nocancel nanosleep_nocancel \
 		   open_nocancel open64_nocancel \
 		   openat_nocancel openat64_nocancel \
-		   pause_nocancel read_nocancel waitpid_nocancel \
-		   write_nocancel statx_cp
+		   pause_nocancel read_nocancel pread64_nocancel \
+		   waitpid_nocancel write_nocancel statx_cp
 
 sysdep_headers += bits/fcntl-linux.h
 
diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index 1ca102a9e2..d385085c61 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -182,6 +182,7 @@  libc {
     __syscall_rt_sigqueueinfo;
     __open_nocancel;
     __read_nocancel;
+    __pread64_nocancel;
     __close_nocancel;
     __sigtimedwait;
     # functions used by nscd
diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
index 16cc31cba5..bf7d80125a 100644
--- a/sysdeps/unix/sysv/linux/not-cancel.h
+++ b/sysdeps/unix/sysv/linux/not-cancel.h
@@ -43,6 +43,9 @@  __typeof (openat64) __openat64_nocancel;
 /* Non cancellable read syscall.  */
 __typeof (__read) __read_nocancel;
 
+/* Non cancellable pread syscall (LFS version).  */
+__typeof (__pread64) __pread64_nocancel;
+
 /* Uncancelable write.  */
 __typeof (__write) __write_nocancel;
 
@@ -84,6 +87,7 @@  hidden_proto (__open64_nocancel)
 hidden_proto (__openat_nocancel)
 hidden_proto (__openat64_nocancel)
 hidden_proto (__read_nocancel)
+hidden_proto (__pread64_nocancel)
 hidden_proto (__write_nocancel)
 hidden_proto (__close_nocancel)
 hidden_proto (__waitpid_nocancel)
diff --git a/sysdeps/unix/sysv/linux/pread64_nocancel.c b/sysdeps/unix/sysv/linux/pread64_nocancel.c
new file mode 100644
index 0000000000..dab61260e5
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/pread64_nocancel.c
@@ -0,0 +1,32 @@ 
+/* Linux pread64() syscall implementation -- non-cancellable.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+#include <sysdep-cancel.h>
+#include <not-cancel.h>
+
+#ifndef __NR_pread64
+# define __NR_pread64 __NR_pread
+#endif
+
+ssize_t
+__pread64_nocancel (int fd, void *buf, size_t count, off64_t offset)
+{
+  return INLINE_SYSCALL_CALL (pread64, fd, buf, count, SYSCALL_LL64_PRW (offset));
+}
+hidden_def (__pread64_nocancel)