public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	libc-alpha@sourceware.org
Subject: Re: [PATCH v2 06/15] linux: Add process_madvise
Date: Tue, 31 May 2022 18:03:23 -0400	[thread overview]
Message-ID: <ae092de1-73a1-d26a-c6b6-f7a3e74afe3e@redhat.com> (raw)
In-Reply-To: <20220207174431.360355-7-adhemerval.zanella@linaro.org>

On 2/7/22 12:44, Adhemerval Zanella via Libc-alpha wrote:
> It was added on Linux 5.10 (ecb8ac8b1f146915aa6b96449b66dd48984caacc)
> with the same functionality as madvise but using a pidfd of the target
> process.

Would you mind splitting out patch 6 and 7 with the suggestions below and I'll
do a final review of that? Thank you.

Comments below.

> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  NEWS                                          |   3 +
>  bits/mman_ext.h                               |  21 ++++
>  misc/sys/mman.h                               |   3 +
>  posix/Makefile                                |   2 +-
>  sysdeps/unix/sysv/linux/Makefile              |   5 +
>  sysdeps/unix/sysv/linux/Versions              |   1 +
>  sysdeps/unix/sysv/linux/aarch64/libc.abilist  |   1 +
>  sysdeps/unix/sysv/linux/alpha/libc.abilist    |   1 +
>  sysdeps/unix/sysv/linux/arc/libc.abilist      |   1 +
>  sysdeps/unix/sysv/linux/arm/be/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/arm/le/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/bits/mman_ext.h       |  33 ++++++
>  sysdeps/unix/sysv/linux/csky/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/hppa/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/i386/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/ia64/libc.abilist     |   1 +
>  .../sysv/linux/m68k/coldfire/libc.abilist     |   1 +
>  .../unix/sysv/linux/m68k/m680x0/libc.abilist  |   1 +
>  .../sysv/linux/microblaze/be/libc.abilist     |   1 +
>  .../sysv/linux/microblaze/le/libc.abilist     |   1 +
>  .../sysv/linux/mips/mips32/fpu/libc.abilist   |   1 +
>  .../sysv/linux/mips/mips32/nofpu/libc.abilist |   1 +
>  .../sysv/linux/mips/mips64/n32/libc.abilist   |   1 +
>  .../sysv/linux/mips/mips64/n64/libc.abilist   |   1 +
>  sysdeps/unix/sysv/linux/nios2/libc.abilist    |   1 +
>  sysdeps/unix/sysv/linux/or1k/libc.abilist     |   1 +
>  .../linux/powerpc/powerpc32/fpu/libc.abilist  |   1 +
>  .../powerpc/powerpc32/nofpu/libc.abilist      |   1 +
>  .../linux/powerpc/powerpc64/be/libc.abilist   |   1 +
>  .../linux/powerpc/powerpc64/le/libc.abilist   |   1 +
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |   1 +
>  .../unix/sysv/linux/riscv/rv64/libc.abilist   |   1 +
>  .../unix/sysv/linux/s390/s390-32/libc.abilist |   1 +
>  .../unix/sysv/linux/s390/s390-64/libc.abilist |   1 +
>  sysdeps/unix/sysv/linux/sh/be/libc.abilist    |   1 +
>  sysdeps/unix/sysv/linux/sh/le/libc.abilist    |   1 +
>  .../sysv/linux/sparc/sparc32/libc.abilist     |   1 +
>  .../sysv/linux/sparc/sparc64/libc.abilist     |   1 +
>  sysdeps/unix/sysv/linux/syscalls.list         |   1 +
>  sysdeps/unix/sysv/linux/tst-process_madvise.c | 107 ++++++++++++++++++
>  .../unix/sysv/linux/x86_64/64/libc.abilist    |   1 +
>  .../unix/sysv/linux/x86_64/x32/libc.abilist   |   1 +
>  42 files changed, 208 insertions(+), 1 deletion(-)
>  create mode 100644 bits/mman_ext.h
>  create mode 100644 sysdeps/unix/sysv/linux/bits/mman_ext.h
>  create mode 100644 sysdeps/unix/sysv/linux/tst-process_madvise.c
> 
> diff --git a/NEWS b/NEWS
> index d882d46842..47cab5d5c1 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -13,6 +13,9 @@ Major new features:
>    have been added.  The pidfd functionality help to solve the issue of PID
>    reuse in Unix systems.
>  
> +* On Linux, the process_madvise has been added.  It is has the same
> +  functionality as madvise but using a pidfd of the target process.

Suggest:

* On Linux, the process_madvise function has been added. It has the same functionality
  as madvise but alters the target process identified by the pidfd.

> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>    [Add deprecations, removals and changes affecting compatibility here]
> diff --git a/bits/mman_ext.h b/bits/mman_ext.h
> new file mode 100644
> index 0000000000..3ac42ab606
> --- /dev/null
> +++ b/bits/mman_ext.h
> @@ -0,0 +1,21 @@
> +/* System-specific extensions of <sys/mman.h>, generic version.

OK.

> +   Copyright (C) 2022 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never include <bits/mman_ext.h> directly; use <sys/mman.h> instead."
> +#endif

OK.

> diff --git a/misc/sys/mman.h b/misc/sys/mman.h
> index 311d44cfb1..02896a0acc 100644
> --- a/misc/sys/mman.h
> +++ b/misc/sys/mman.h
> @@ -146,6 +146,9 @@ extern int shm_open (const char *__name, int __oflag, mode_t __mode);
>  /* Remove shared memory segment.  */
>  extern int shm_unlink (const char *__name);
>  
> +/* System-specific extensions.  */
> +#include <bits/mman_ext.h>

OK. Include bits.

> +
>  __END_DECLS
>  
>  #endif	/* sys/mman.h */
> diff --git a/posix/Makefile b/posix/Makefile
> index cfebb8ef06..d1df7c27cb 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -32,7 +32,7 @@ headers	:= sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h	      \
>  	   bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h	      \
>  	   bits/sched.h bits/cpu-set.h re_comp.h wait.h bits/environments.h   \
>  	   cpio.h spawn.h bits/unistd.h bits/types/struct_sched_param.h	      \
> -	   bits/unistd_ext.h bits/types/idtype_t.h
> +	   bits/unistd_ext.h bits/types/idtype_t.h bits/mman_ext.h

OK. New public header.

>  
>  routines :=								      \
>  	uname								      \
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 6fefcd8fe7..2c7c425ab7 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -130,6 +130,11 @@ tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
>    tst-pidfd \
>    # tests
>  
> +# process_madvise requires CAP_SYS_ADMIN.
> +xtests += \
> +  tst-process_madvise \
> +  # tests-container

Wrong closing "#" should say "# xtests"

> +
>  # Test for the symbol version of fcntl that was replaced in glibc 2.28.
>  ifeq ($(have-GLIBC_2.27)$(build-shared),yesyes)
>  tests += tst-ofdlocks-compat
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index 694c64a5bc..c13a3c4fe7 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -303,6 +303,7 @@ libc {
>      pidfd_open;
>      pidfd_getfd;
>      pidfd_send_signal;
> +    process_madvise;

OK.

>    }
>    GLIBC_PRIVATE {
>      # functions used in other libraries
> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> index 7602129631..4d8e64ce04 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
> @@ -2619,3 +2619,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
> index 5b39d6023c..36f3129257 100644
> --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
> @@ -2716,6 +2716,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/arc/libc.abilist b/sysdeps/unix/sysv/linux/arc/libc.abilist
> index e413302f2e..2a1e346d4b 100644
> --- a/sysdeps/unix/sysv/linux/arc/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/arc/libc.abilist
> @@ -2380,3 +2380,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
> index ade1b93d13..25de8a018c 100644
> --- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
> @@ -499,6 +499,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _Exit F
>  GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
>  GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
> diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
> index 10e91537fa..e9cf9b1593 100644
> --- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
> @@ -496,6 +496,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _Exit F
>  GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
>  GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
> diff --git a/sysdeps/unix/sysv/linux/bits/mman_ext.h b/sysdeps/unix/sysv/linux/bits/mman_ext.h
> new file mode 100644
> index 0000000000..20960e7df5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/mman_ext.h
> @@ -0,0 +1,33 @@
> +/* System-specific extensions of <sys/mman.h>, Linux version.
> +   Copyright (C) 2022 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_MMAN_H
> +# error "Never include <bits/mman_ext.h> directly; use <sys/mman.h> instead."
> +#endif
> +
> +#ifdef __USE_GNU
> +#include <bits/types/struct_iovec.h>
> +
> +/*
> + */
> +extern __ssize_t process_madvise (int __pid_fd, const struct iovec* __iov,
> +				  size_t __count, int __advice,
> +				  unsigned __flags)

OK. Reviewing order of arguments:

1. int pidfd [OK]
2. const struct iovec* iovec [OK]
3. size_t vlen [OK] (count for iovec <= IOV_MAX)
4. int advice [OK]
5. unsigned int flags [OK]

Looks good.


> +  __THROW;
> +
> +#endif /* __USE_GNU  */
> diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist
> index 22618c7aaa..66c2f28c43 100644
> --- a/sysdeps/unix/sysv/linux/csky/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/csky/libc.abilist
> @@ -2655,3 +2655,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
> index e2aea90c74..527880dc06 100644
> --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
> @@ -2604,6 +2604,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
> index 29ecca26bb..1497b8a2c3 100644
> --- a/sysdeps/unix/sysv/linux/i386/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
> @@ -2788,6 +2788,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
> index 69a2ca2391..abe351d0b4 100644
> --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
> @@ -2554,6 +2554,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> index 23cc4ddd8a..130f7a6b04 100644
> --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
> @@ -500,6 +500,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _Exit F
>  GLIBC_2.4 _IO_2_1_stderr_ D 0x98
>  GLIBC_2.4 _IO_2_1_stdin_ D 0x98
> diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> index 2d1d8a3d0c..cf59edb383 100644
> --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
> @@ -2731,6 +2731,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
> index 7b6e88bdb9..f1b927d63d 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
> @@ -2704,3 +2704,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
> index 155c974790..9bb7c76f94 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
> @@ -2701,3 +2701,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> index 7a811120ea..03e7820eea 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
> @@ -2696,6 +2696,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> index 7d28dca5c1..3d70dc1140 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
> @@ -2694,6 +2694,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> index 41790f0d63..5498e2ee30 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
> @@ -2702,6 +2702,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> index 9c15006c12..dbebed5d36 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
> @@ -2605,6 +2605,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
> index 27c0498b60..a0bfa86068 100644
> --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
> @@ -2743,3 +2743,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/or1k/libc.abilist b/sysdeps/unix/sysv/linux/or1k/libc.abilist
> index e500d4756f..7d09a03d0e 100644
> --- a/sysdeps/unix/sysv/linux/or1k/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/or1k/libc.abilist
> @@ -2126,3 +2126,4 @@ GLIBC_2.35 wscanf F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> index adb21db204..db8fc28e75 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
> @@ -2758,6 +2758,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> index 28a08c4afb..e6cbb83b26 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
> @@ -2791,6 +2791,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> index 98c5f3e61c..162241fc68 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
> @@ -2513,6 +2513,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> index 31e5aa90cd..2845ee2015 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
> @@ -2815,3 +2815,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> index 87d90bf668..277f6fbe95 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> @@ -2382,3 +2382,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> index b4a8f56aa2..6f2a4d4504 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> @@ -2582,3 +2582,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> index e4a58cc145..bfb317488d 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
> @@ -2756,6 +2756,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> index ef0f0c3fa1..beef516979 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
> @@ -2550,6 +2550,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
> index 603d50668f..c326c9320a 100644
> --- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
> @@ -2611,6 +2611,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
> index 078cdf05a7..704f489a8e 100644
> --- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
> @@ -2608,6 +2608,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> index 735505ac51..b9e8a34153 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
> @@ -2751,6 +2751,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 _IO_fprintf F
>  GLIBC_2.4 _IO_printf F
>  GLIBC_2.4 _IO_sprintf F
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> index 48df7952cd..e921a8dd76 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
> @@ -2577,6 +2577,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index d9653bb433..6942a16d0c 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -43,6 +43,7 @@ pidfd_open	EXTRA	pidfd_open	i:iU	pidfd_open
>  pidfd_getfd	EXTRA	pidfd_getfd	i:iiU	pidfd_getfd
>  pivot_root	EXTRA	pivot_root	i:ss	pivot_root
>  pidfd_send_signal	EXTRA	pidfd_send_signal	i:iiPU	pidfd_send_signal
> +process_madvise EXTRA   process_madvise i:iPiiU process_madvise

Return: ssize_t
Args: int, pointer, size_t, int, unsigned int.

Should it be "i:iPniU" ?

I don't think we use 'n' for anything, but in theory that argument
is the scalar size of thes struct iovec *.

>  query_module	EXTRA	query_module	i:sipip	__compat_query_module	query_module@GLIBC_2.0:GLIBC_2.23
>  quotactl	EXTRA	quotactl	i:isip	quotactl
>  remap_file_pages -	remap_file_pages i:pUiUi	__remap_file_pages remap_file_pages
> diff --git a/sysdeps/unix/sysv/linux/tst-process_madvise.c b/sysdeps/unix/sysv/linux/tst-process_madvise.c
> new file mode 100644
> index 0000000000..bbb04951f8
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-process_madvise.c
> @@ -0,0 +1,107 @@
> +/* Basic tests for Linux process_madvise.
> +   Copyright (C) 2022 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <support/check.h>
> +#include <support/process_state.h>
> +#include <support/xunistd.h>
> +#include <support/xsocket.h>
> +#include <sys/pidfd.h>
> +#include <sys/mman.h>
> +#include <sys/wait.h>
> +
> +/* The pair of sockets used for coordination.  The subprocess uses
> +   sockets[1].  */
> +static int sockets[2];
> +
> +static long int page_size;
> +
> +static void
> +subprocess (void)
> +{
> +  void *p1 = xmmap (NULL, page_size * 2, PROT_READ | PROT_WRITE,
> +		    MAP_PRIVATE | MAP_ANONYMOUS, -1);
> +
> +  void *p2 = xmmap (NULL, page_size, PROT_READ | PROT_WRITE,
> +		    MAP_PRIVATE | MAP_ANONYMOUS, -1);
> +  xmunmap(p2, page_size);
> +
> +  xsendto (sockets[1], &(struct iovec) { p1, page_size * 2 },
> +	   sizeof (struct iovec), 0, NULL, 0);
> +
> +  xsendto (sockets[1], &(struct iovec) { p2, page_size },
> +	   sizeof (struct iovec), 0, NULL, 0);
> +
> +  pause ();
> +
> +  _exit (0);
> +}
> +

We should have some comments that explain the intent of the test.

Suggestions below.

> +static int
> +do_test (void)
> +{
> +  page_size = sysconf (_SC_PAGE_SIZE);
> +
> +  {
> +    int r = pidfd_open (-1, 0);
> +    TEST_COMPARE (r, -1);
> +    if (errno == ENOSYS)
> +      FAIL_UNSUPPORTED ("kernel does not support pidfd_open, skipping test");
> +  }
> +
> +  TEST_COMPARE (socketpair (AF_UNIX, SOCK_STREAM, 0, sockets), 0);
> +

May we please add a test for EBADF with an invalid pidfd?

> +  pid_t pid = xfork ();
> +  if (pid == 0)
> +    {
> +      xclose (sockets[0]);
> +      subprocess ();
> +    }
> +  xclose (sockets[1]);
> +
> +  int pidfd = pidfd_open (pid, 0);
> +  TEST_VERIFY (pidfd != -1);
> +
> +  {

Suggest:

/* The target process is going to send us two iovec's. The first one
   points to a valid mapping, the other points to a previously valid
   mapping which has now been unmapped.  */

> +    struct iovec iv;
> +    xrecvfrom (sockets[0], &iv, sizeof (iv), 0, NULL, 0);
> +

/* We expect this to succeed in the target process because the mapping
   is valid.  */

> +    TEST_COMPARE (process_madvise (pidfd, &iv, 1, MADV_COLD, 0), 2 * page_size);
> +  }
> +
> +  {
> +    struct iovec iv;
> +    xrecvfrom (sockets[0], &iv, sizeof (iv), 0, NULL, 0);
> +

/* We expect this to fail in the target process because the second
   iovec points to an unmapped region. Thie target process arranges
   for this to be the case.  */

> +    TEST_COMPARE (process_madvise (pidfd, &iv, 1, MADV_COLD, 0), -1);
> +    TEST_COMPARE (errno, ENOMEM);

Is this correct?

I did not expect ENOMEM for an unmapped region. Maybe EFAULT?

> +  }
> +

May we please add a test for EINVAL when count is too large?

> +  TEST_COMPARE (pidfd_send_signal (pidfd, SIGKILL, NULL, 0), 0);
> +  {
> +    siginfo_t info;
> +    int r = waitid (P_PIDFD, pidfd, &info, WEXITED);
> +    TEST_COMPARE (r, 0);
> +    TEST_COMPARE (info.si_status, SIGKILL);
> +    TEST_COMPARE (info.si_code, CLD_KILLED);
> +  }

May we please add a test for ESRCH when target process is gone?

This way we cover the whole life-cycle of the process.

> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> index 1629fb7762..8948dc2705 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
> @@ -2528,6 +2528,7 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F
>  GLIBC_2.4 __confstr_chk F
>  GLIBC_2.4 __fgets_chk F
>  GLIBC_2.4 __fgets_unlocked_chk F
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> index ee9674ff75..0988804f24 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
> @@ -2634,3 +2634,4 @@ GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
>  GLIBC_2.36 pidfd_getfd F
>  GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
> +GLIBC_2.36 process_madvise F


-- 
Cheers,
Carlos.


  reply	other threads:[~2022-05-31 22:03 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 17:44 [PATCH v2 00/15] linux: Add new syscalls Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 01/15] linux: Add pidfd_open Adhemerval Zanella
2022-05-16 21:12   ` Carlos O'Donell
2022-06-01 18:02   ` Matheus Castanho
2022-06-01 18:49     ` Adhemerval Zanella
2022-06-01 20:53       ` Matheus Castanho
2022-06-01 22:22         ` Joseph Myers
2022-02-07 17:44 ` [PATCH v2 02/15] linux: Add pidfd_getfd Adhemerval Zanella
2022-05-16 21:12   ` Carlos O'Donell
2022-02-07 17:44 ` [PATCH v2 03/15] linux: Add pidfd_send_signal Adhemerval Zanella
2022-05-16 21:12   ` Carlos O'Donell
2022-02-07 17:44 ` [PATCH v2 04/15] linux: Add P_PIDFD Adhemerval Zanella
2022-05-16 21:12   ` Carlos O'Donell
2022-02-07 17:44 ` [PATCH v2 05/15] linux: Add tst-pidfd.c Adhemerval Zanella
2022-05-16 21:12   ` Carlos O'Donell
2022-02-07 17:44 ` [PATCH v2 06/15] linux: Add process_madvise Adhemerval Zanella
2022-05-31 22:03   ` Carlos O'Donell [this message]
2022-06-01 18:04     ` Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 07/15] linux: Add process_mrelease Adhemerval Zanella
2022-05-31 22:12   ` Carlos O'Donell
2022-06-01 18:35     ` Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 08/15] linux: Add fsopen Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 09/15] linux: Add fsmount Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 10/15] linux: Add move_mount Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 11/15] linux: Add fsconfig Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 12/15] linux: Add fspick Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 13/15] linux: Add open_tree Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 14/15] linux: Add tst-mount to check for Linux new mount API Adhemerval Zanella
2022-02-07 17:44 ` [PATCH v2 15/15] linux: Add mount_setattr Adhemerval Zanella
2022-05-16 21:12 ` [PATCH v2 00/15] linux: Add new syscalls Carlos O'Donell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae092de1-73a1-d26a-c6b6-f7a3e74afe3e@redhat.com \
    --to=carlos@redhat.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).