Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH v28 07/12] LRNG - add kernel crypto API PRNG extension
From: Stephan Mueller @ 2020-01-16  6:54 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
	Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
	Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
	Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
	Andy Lutomirski, Florian Weimer, Lennart Poettering,
	Nicolai Stange
In-Reply-To: <d98e7a45-3d1b-8119-1ed0-87aea0f3c6f3@infradead.org>

Am Donnerstag, 16. Januar 2020, 01:15:46 CET schrieb Randy Dunlap:

Hi Randy,

> On 1/15/20 2:34 AM, Stephan Müller wrote:
> > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > CC: Willy Tarreau <w@1wt.eu>
> > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > CC: Vito Caputo <vcaputo@pengaru.com>
> > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > CC: Jan Kara <jack@suse.cz>
> > CC: Ray Strode <rstrode@redhat.com>
> > CC: William Jon McCann <mccann@jhu.edu>
> > CC: zhangjs <zachary@baishancloud.com>
> > CC: Andy Lutomirski <luto@kernel.org>
> > CC: Florian Weimer <fweimer@redhat.com>
> > CC: Lennart Poettering <mzxreary@0pointer.de>
> > CC: Nicolai Stange <nstange@suse.de>
> > Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Tested-by: Neil Horman <nhorman@redhat.com>
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > 
> >  drivers/char/lrng/Kconfig      |  10 +
> >  drivers/char/lrng/Makefile     |   1 +
> >  drivers/char/lrng/lrng_kcapi.c | 327 +++++++++++++++++++++++++++++++++
> >  3 files changed, 338 insertions(+)
> >  create mode 100644 drivers/char/lrng/lrng_kcapi.c
> > 
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > index 15fb623d9d1f..0d070a3897dd 100644
> > --- a/drivers/char/lrng/Kconfig
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -80,6 +80,16 @@ config LRNG_DRBG
> > 
> >  	  Enable the SP800-90A DRBG support for the LRNG. Once the
> >  	  module is loaded, output from /dev/random, /dev/urandom,
> >  	  getrandom(2), or get_random_bytes is provided by a DRBG.
> > 
> > +
> > +config LRNG_KCAPI
> > +	tristate "Kernel Crypto API support for the LRNG"
> > +	select CRYPTO_RNG
> 
> Don't select CRYPTO_RNG unless you know that CRYPTO is set/enabled.

I added "depends on CRYPTO"
> 
> > +	help
> > +	  Enable the support for generic pseudo-random number
> > +	  generators offered by the kernel crypto API with the
> > +	  LRNG. Once the module is loaded, output from /dev/random,
> > +	  /dev/urandom, getrandom(2), or get_random_bytes is
> > +	  provided by the selected kernel crypto API RNG.
> > 
> >  endif # LRNG_DRNG_SWITCH
> >  
> >  endif # LRNG

Thank you.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v28 06/12] LRNG - add SP800-90A DRBG extension
From: Stephan Mueller @ 2020-01-16  6:55 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
	Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
	Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
	Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
	Andy Lutomirski, Florian Weimer, Lennart Poettering,
	Nicolai Stange
In-Reply-To: <55a89cdf-e36d-fb36-29fa-46ebfb139294@infradead.org>

Am Donnerstag, 16. Januar 2020, 01:14:35 CET schrieb Randy Dunlap:

Hi Randy,

> On 1/15/20 2:33 AM, Stephan Müller wrote:
> > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > CC: Willy Tarreau <w@1wt.eu>
> > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > CC: Vito Caputo <vcaputo@pengaru.com>
> > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > CC: Jan Kara <jack@suse.cz>
> > CC: Ray Strode <rstrode@redhat.com>
> > CC: William Jon McCann <mccann@jhu.edu>
> > CC: zhangjs <zachary@baishancloud.com>
> > CC: Andy Lutomirski <luto@kernel.org>
> > CC: Florian Weimer <fweimer@redhat.com>
> > CC: Lennart Poettering <mzxreary@0pointer.de>
> > CC: Nicolai Stange <nstange@suse.de>
> > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Tested-by: Neil Horman <nhorman@redhat.com>
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > 
> >  drivers/char/lrng/Kconfig     |  11 ++
> >  drivers/char/lrng/Makefile    |   1 +
> >  drivers/char/lrng/lrng_drbg.c | 260 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 272 insertions(+)
> >  create mode 100644 drivers/char/lrng/lrng_drbg.c
> > 
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > index cb701bb0b8b6..15fb623d9d1f 100644
> > --- a/drivers/char/lrng/Kconfig
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -71,4 +71,15 @@ menuconfig LRNG_DRNG_SWITCH
> > 
> >  	  accessible via the external interfaces. With this configuration
> >  	  option other DRNGs can be selected and loaded at runtime.
> > 
> > +if LRNG_DRNG_SWITCH
> > +config LRNG_DRBG
> > +	tristate "SP800-90A support for the LRNG"
> > +	select CRYPTO_DRBG_MENU
> > +	select CRYPTO_CMAC if CRYPTO_DRBG_CTR
> 
> Don't select these if CRYPTO is not already set/enabled.
> It causes Kconfig warnings and possible build errors.

I added "depends on CRYPTO"
> 
> > +	help
> > +	  Enable the SP800-90A DRBG support for the LRNG. Once the
> > +	  module is loaded, output from /dev/random, /dev/urandom,
> > +	  getrandom(2), or get_random_bytes is provided by a DRBG.
> > +endif # LRNG_DRNG_SWITCH
> > +
> > 
> >  endif # LRNG
> > 
> > diff --git a/drivers/char/lrng/lrng_drbg.c b/drivers/char/lrng/lrng_drbg.c
> > new file mode 100644
> > index 000000000000..8bf2badb1fe0
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_drbg.c
> > @@ -0,0 +1,260 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +/*
> > + * Backend for the LRNG providing the cryptographic primitives using the
> > + * kernel crypto API and its DRBG.
> > + *
> > + * Copyright (C) 2016 - 2020, Stephan Mueller <smueller@chronox.de>
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <crypto/drbg.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/lrng.h>
> > +
> > +/*
> > + * Define a DRBG plus a hash / MAC used to extract data from the entropy
> > pool. + * For LRNG_HASH_NAME you can use a hash or a MAC (HMAC or CMAC)
> > of your choice + * (Note, you should use the suggested selections below
> > -- using SHA-1 or MD5 + * is not wise). The idea is that the used cipher
> > primitive can be selected to + * be the same as used for the DRBG. I.e.
> > the LRNG only uses one cipher + * primitive using the same cipher
> > implementation with the options offered in + * the following. This means,
> > if the CTR DRBG is selected and AES-NI is present, + * both the CTR DRBG
> > and the selected cmac(aes) use AES-NI.
> > + *
> > + * The security strengths of the DRBGs are all 256 bits according to
> > + * SP800-57 section 5.6.1.
> > + *
> > + * This definition is allowed to be changed.
> > + */
> > +#ifdef CONFIG_CRYPTO_DRBG_CTR
> > +static unsigned int lrng_drbg_type = 0;
> > +#elif defined CONFIG_CRYPTO_DRBG_HMAC
> > +static unsigned int lrng_drbg_type = 1;
> > +#elif defined CONFIG_CRYPTO_DRBG_HASH
> > +static unsigned int lrng_drbg_type = 2;
> > +#else
> > +#error "Unknown DRBG in use"
> > +#endif
> > +
> > +/* The parameter must be r/o in sysfs as otherwise races appear. */
> > +module_param(lrng_drbg_type, uint, 0444);
> > +MODULE_PARM_DESC(lrng_drbg_type, "DRBG type used for LRNG (0->CTR_DRBG, "
> > +				 "1->HMAC_DRBG, 2->Hash_DRBG)");
> 
> One line for the string, please, not split.

Changed

Thank you.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v28 01/12] Linux Random Number Generator
From: Stephan Mueller @ 2020-01-16  7:22 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Arnd Bergmann, Greg Kroah-Hartman,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, LKML,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
	Alexander E. Patrakov, Ahmed S. Darwish, Theodore Y. Ts'o,
	Willy Tarreau, Matthew Garrett, Vito Caputo, Andreas Dilger,
	Jan Kara, Ray Strode, William Jon McCann, zhangjs,
	Andy Lutomirski, Florian Weimer, Lennart Poettering,
	Nicolai Stange
In-Reply-To: <3a8d5d2d-d54f-cf18-0c93-dbe8cd91ed12-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

Am Donnerstag, 16. Januar 2020, 01:11:40 CET schrieb Randy Dunlap:

Hi Randy,

> Hi,
> 
> On 1/15/20 2:31 AM, Stephan Müller wrote:
> > CC: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> > CC: "Alexander E. Patrakov" <patrakov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > CC: "Ahmed S. Darwish" <darwish.07-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > CC: "Theodore Y. Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
> > CC: Willy Tarreau <w@1wt.eu>
> > CC: Matthew Garrett <mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
> > CC: Vito Caputo <vcaputo-IiWei5kqaphBDgjK7y7TUQ@public.gmane.org>
> > CC: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
> > CC: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> > CC: Ray Strode <rstrode-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > CC: William Jon McCann <mccann-4GNroTWusrE@public.gmane.org>
> > CC: zhangjs <zachary-+jQ8zvd9CV5SzTKKI/wJ2A@public.gmane.org>
> > CC: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > CC: Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > CC: Lennart Poettering <mzxreary-uLTowLwuiw4b1SvskN2V4Q@public.gmane.org>
> > CC: Nicolai Stange <nstange-l3A5Bk7waGM@public.gmane.org>
> > Mathematical aspects Reviewed-by: "Peter, Matthias"
> > <matthias.peter-AKRvH3fzEZsb1SvskN2V4Q@public.gmane.org> Reviewed-by: Marcelo Henrique Cerri
> > <marcelo.cerri-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> > Reviewed-by: Roman Drahtmueller <draht-UTA6Eu5RHcbyVfKhlJ+g5A@public.gmane.org>
> > Tested-by: Roman Drahtmüller <draht-UTA6Eu5RHcbyVfKhlJ+g5A@public.gmane.org>
> > Tested-by: Marcelo Henrique Cerri <marcelo.cerri-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> > Tested-by: Neil Horman <nhorman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
> > ---
> > 
> >  MAINTAINERS                         |   7 +
> >  drivers/char/Kconfig                |   2 +
> >  drivers/char/Makefile               |   9 +-
> >  drivers/char/lrng/Kconfig           |  67 +++
> >  drivers/char/lrng/Makefile          |   9 +
> >  drivers/char/lrng/lrng_archrandom.c |  94 ++++
> >  drivers/char/lrng/lrng_aux.c        | 148 +++++++
> >  drivers/char/lrng/lrng_chacha20.c   | 265 ++++++++++++
> >  drivers/char/lrng/lrng_chacha20.h   |  25 ++
> >  drivers/char/lrng/lrng_drng.c       | 400 +++++++++++++++++
> >  drivers/char/lrng/lrng_interfaces.c | 638 ++++++++++++++++++++++++++++
> >  drivers/char/lrng/lrng_internal.h   | 296 +++++++++++++
> >  drivers/char/lrng/lrng_lfsr.h       | 152 +++++++
> >  drivers/char/lrng/lrng_pool.c       | 588 +++++++++++++++++++++++++
> >  drivers/char/lrng/lrng_sw_noise.c   | 102 +++++
> >  drivers/char/lrng/lrng_sw_noise.h   |  57 +++
> >  include/linux/lrng.h                |  63 +++
> >  17 files changed, 2921 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/char/lrng/Kconfig
> >  create mode 100644 drivers/char/lrng/Makefile
> >  create mode 100644 drivers/char/lrng/lrng_archrandom.c
> >  create mode 100644 drivers/char/lrng/lrng_aux.c
> >  create mode 100644 drivers/char/lrng/lrng_chacha20.c
> >  create mode 100644 drivers/char/lrng/lrng_chacha20.h
> >  create mode 100644 drivers/char/lrng/lrng_drng.c
> >  create mode 100644 drivers/char/lrng/lrng_interfaces.c
> >  create mode 100644 drivers/char/lrng/lrng_internal.h
> >  create mode 100644 drivers/char/lrng/lrng_lfsr.h
> >  create mode 100644 drivers/char/lrng/lrng_pool.c
> >  create mode 100644 drivers/char/lrng/lrng_sw_noise.c
> >  create mode 100644 drivers/char/lrng/lrng_sw_noise.h
> >  create mode 100644 include/linux/lrng.h
> > 
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > new file mode 100644
> > index 000000000000..56f13efd3592
> > --- /dev/null
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Linux Random Number Generator configuration
> > +#
> > +
> > +menuconfig LRNG
> > +	bool "Linux Random Number Generator"
> 
> This should probably depend on CRYPTO and/or some other CRYPTO_xxx symbols.
> Or (worst case) select them.  :(

It does not - all CRYPTO related code is limited into the lrng_drbg.c, 
lrng_kcapi.c and lrng_jent.c which are all guarded by a Kconfig option.
> 
> This message (when CONFIG_CRYPTO is disabled and no crypto facilities are
> enabled) should be avoidable when the correct Kconfig entries are used:
> 
> ../drivers/char/lrng/lrng_drbg.c:38:2: error: #error "Unknown DRBG in use"
>  #error "Unknown DRBG in use"

Right - this should now be fixed with a depends on CRYPTO for the 
aforementioned 3 C files which have an equal entry in Kconfig.
> 
> > +	help
> > +	  The Linux Random Number Generator (LRNG) is the replacement
> > +	  of the existing /dev/random provided with drivers/char/random.c.
> > +	  It generates entropy from different noise sources and
> > +	  delivers significant entropy during boot.
> > +
> > +if LRNG
> > +
> > +choice
> > +	prompt "LRNG Entropy Pool Size"
> > +	default LRNG_POOL_SIZE_4096
> > +	help
> > +	  Select the size of the LRNG entropy pool. The size of the
> > +	  entropy pool is relevant for the amount of entropy that
> > +	  the LRNG can maintain as a maximum. The larger the size
> > +	  of the entropy pool is the more entropy can be maintained
> > +	  but the less often older entropic values are overwritten
> > +	  with new entropy.
> > +
> > +	config LRNG_POOL_SIZE_512
> > +		bool "512 bits"
> > +
> > +	config LRNG_POOL_SIZE_1024
> > +		bool "1024 bits"
> > +
> > +	config LRNG_POOL_SIZE_2048
> > +		bool "2048 bits"
> > +
> > +	config LRNG_POOL_SIZE_4096
> > +		bool "4096 bits (default)"
> > +
> > +	config LRNG_POOL_SIZE_8192
> > +		bool "8192 bits"
> > +
> > +	config LRNG_POOL_SIZE_16384
> > +		bool "16384 bits"
> > +
> > +	config LRNG_POOL_SIZE_32768
> > +		bool "32768 bits"
> > +
> > +	config LRNG_POOL_SIZE_65536
> > +		bool "65536 bits"
> > +
> > +	config LRNG_POOL_SIZE_131072
> > +		bool "131072 bits"
> > +endchoice
> > +
> > +config LRNG_POOL_SIZE
> > +	int
> > +	default 0 if LRNG_POOL_SIZE_512
> > +	default 1 if LRNG_POOL_SIZE_1024
> > +	default 2 if LRNG_POOL_SIZE_2048
> > +	default 3 if LRNG_POOL_SIZE_4096
> > +	default 4 if LRNG_POOL_SIZE_8192
> > +	default 5 if LRNG_POOL_SIZE_16384
> > +	default 6 if LRNG_POOL_SIZE_32768
> > +	default 7 if LRNG_POOL_SIZE_65536
> > +	default 8 if LRNG_POOL_SIZE_131072
> > +
> > +endif # LRNG
> > 
> > diff --git a/drivers/char/lrng/lrng_archrandom.c
> > b/drivers/char/lrng/lrng_archrandom.c new file mode 100644
> > index 000000000000..eeba708d025f
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_archrandom.c
> > @@ -0,0 +1,94 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +/*
> > + * LRNG Fast Noise Source: CPU-based noise source
> > + *
> > + * Copyright (C) 2016 - 2020, Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/random.h>
> > +
> > +#include "lrng_internal.h"
> > +
> > +/*
> > + * Estimated entropy of data is a 32th of
> > LRNG_DRNG_SECURITY_STRENGTH_BITS. + * As we have no ability to review the
> > implementation of those noise sources, + * it is prudent to have a
> > conservative estimate here.
> > + */
> > +#define LRNG_ARCHRANDOM_DEFAULT_STRENGTH
> > (LRNG_DRNG_SECURITY_STRENGTH_BITS>>5) +#define
> > LRNG_ARCHRANDOM_TRUST_CPU_STRENGTH LRNG_DRNG_SECURITY_STRENGTH_BITS
> > +#ifdef CONFIG_RANDOM_TRUST_CPU
> > +static u32 archrandom = LRNG_ARCHRANDOM_TRUST_CPU_STRENGTH;
> > +#else
> > +static u32 archrandom = LRNG_ARCHRANDOM_DEFAULT_STRENGTH;
> > +#endif
> > +module_param(archrandom, uint, 0644);
> > +MODULE_PARM_DESC(archrandom, "Entropy in bits of 256 data bits from CPU
> > noise " +			     "source (e.g. RDRAND)");
> 
> Please put the string on one line like several other MODULE_PARM_DESC() are
> done:
> 
> +MODULE_PARM_DESC(archrandom,
> +		"Entropy in bits of 256 data bits from CPU noise source (e.g. 
RDRAND)");

Done.
> 
> 
> With CONFIG_CRYPTO disabled, these warnings happen:
> 
> WARNING: unmet direct dependencies detected for CRYPTO_DRBG_MENU
>   Depends on [n]: CRYPTO [=n]
>   Selected by [m]:
>   - LRNG_DRBG [=m] && LRNG [=y] && LRNG_DRNG_SWITCH [=y]
> 
> WARNING: unmet direct dependencies detected for CRYPTO_RNG
>   Depends on [n]: CRYPTO [=n]
>   Selected by [m]:
>   - LRNG_KCAPI [=m] && LRNG [=y] && LRNG_DRNG_SWITCH [=y]
> 
> ../drivers/char/lrng/lrng_drbg.c: In function ‘lrng_hash_name’:
> ../drivers/char/lrng/lrng_drbg.c:225:1: warning: control reaches end of
> non-void function [-Wreturn-type] }
>  ^
> ../drivers/char/lrng/lrng_drbg.c: In function ‘lrng_drbg_name’:
> ../drivers/char/lrng/lrng_drbg.c:220:1: warning: control reaches end of
> non-void function [-Wreturn-type] }
>  ^
> 
> and build errors happen also, which can be prevented with Kconfig fixes.

With the changes to the Kconfig file as explained in the other emails, I can 
successfully compile the LRNG without the crypto API being enabled.

Thank you for pointing that one out.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] mm/compaction: Disable compact_unevictable_allowed on RT
From: Sebastian Andrzej Siewior @ 2020-01-16 10:22 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: linux-mm, Thomas Gleixner, Andrew Morton, Luis Chamberlain,
	Kees Cook, Iurii Zaikin, Mel Gorman, Linux API
In-Reply-To: <4cf4507b-0632-34e6-5985-df933559af9f@suse.cz>

On 2020-01-15 23:04:19 [+0100], Vlastimil Babka wrote:
> On 1/15/2020 5:10 PM, Sebastian Andrzej Siewior wrote:
> > Since commit
> >     5bbe3547aa3ba ("mm: allow compaction of unevictable pages")
> > 
> > it is allowed to examine mlocked pages and compact them by default.
> > On -RT even minor pagefaults are problematic because it may take a few
> > 100us to resolve them and until then the task is blocked.
> 
> Fine, this makes sense on RT I guess. There might be some trade-off for
> high-order allocation latencies though. We could perhaps migrate such mlocked
> pages to pages allocated without __GFP_MOVABLE during the mlock() to at least
> somewhat prevent them being scattered all over the zones. For MCL_FUTURE,
> allocate them as unmovable from the beginning. But that can wait until issues
> are reported.
> I assume you have similar solution for NUMA balancing and whatever else can
> cause minor faults?

I've found this one while testing. Could you please point to the NUMA
balancing that might be an issue?

> > Make compact_unevictable_allowed = 0 default and remove it from /proc on
> > RT.
> 
> Removing it is maybe going too far in terms of RT kernel differences confusing
> users? Change the default sure, perhaps making it read-only, but removing?

Okay. I will make it RO then. 

> > Link: https://lore.kernel.org/linux-mm/20190710144138.qyn4tuttdq6h7kqx@linutronix.de/
> 
> In any case the sysctl Documentation/ should be updated? And perhaps also the
> mlock manpage as you noted in the older thread above?

Sure. Let me add the sysctl documentation to this patch and then I will
look into the manpage.

> Thanks,
> Vlastimil

Sebastian

^ permalink raw reply

* Re: clone3 on ARC (was Re: [PATCH v3 2/2] arch: wire-up clone3() syscall)
From: Christian Brauner @ 2020-01-16 11:25 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Christian Brauner, Arnd Bergmann, Al Viro,
	Linux Kernel Mailing List, Linus Torvalds, Jann Horn, Kees Cook,
	Florian Weimer, Oleg Nesterov, David Howells, Andrew Morton,
	Adrian Reber, Linux API, linux-arch, the arch/x86 maintainers,
	arcml
In-Reply-To: <a58c8425-83a3-b64c-339a-7e94a72f4bee@synopsys.com>

On Wed, Jan 15, 2020 at 10:41:20PM +0000, Vineet Gupta wrote:
> On 6/4/19 2:29 PM, Christian Brauner wrote:
> > On Tue, Jun 04, 2019 at 08:40:01PM +0200, Arnd Bergmann wrote:
> >> On Tue, Jun 4, 2019 at 6:09 PM Christian Brauner <christian@brauner.io> wrote:
> >>>
> >>> Wire up the clone3() call on all arches that don't require hand-rolled
> >>> assembly.
> >>>
> >>> Some of the arches look like they need special assembly massaging and it is
> >>> probably smarter if the appropriate arch maintainers would do the actual
> >>> wiring. Arches that are wired-up are:
> >>> - x86{_32,64}
> >>> - arm{64}
> >>> - xtensa
> >>
> >> The ones you did look good to me. I would hope that we can do all other
> >> architectures the same way, even if they have special assembly wrappers
> >> for the old clone(). The most interesting cases appear to be ia64, alpha,
> >> m68k and sparc, so it would be good if their maintainers could take a
> >> look.
> > 
> > Yes, agreed. They can sort this out even after this lands.
> > 
> >>
> >> What do you use for testing? Would it be possible to override the
> >> internal clone() function in glibc with an LD_PRELOAD library
> >> to quickly test one of the other architectures for regressions?
> > 
> > I have a test program that is rather horrendously ugly and I compiled
> > kernels for x86 and the arms and tested in qemu. The program basically
> > looks like [1].
> 
> I just got around to fixing this for ARC (patch to follow after we sort out the
> testing) and was trying to use the test case below for a qucik and dirty smoke
> test (so existing toolchain lacking with headers lacking NR_clone3 or struct
> clone_args etc). I did hack those up, but then spotted below
> 
> uapi/linux/sched.h
> 
> |    struct clone_args {
> |	__aligned_u64 flags;
> |	__aligned_u64 pidfd;
> |	__aligned_u64 child_tid;
> |	__aligned_u64 parent_tid;
> ..
> ..
> 
> Are all clone3 arg fields supposed to be 64-bit wide, even things like @child_tid,
> @tls .... which are traditionally ARCH word wide ?

This is just the kernel ABI we expose to userspace with the intention to
make it easy for us to handle 32 and 64 bit. A libc like glibc is
expected to expose a properly typed struct to userspace. The kernel
struct kernel_clone_args has "correct" typing.

Christian

^ permalink raw reply

* Re: [PATCH v2 2/3] clone3: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-16 12:29 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Oleg Nesterov,
	Johannes Weiner, Li Zefan, Peter Zijlstra,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200108180906.l4mvtdmh7nm2z7sc@wittgenstein>

On Wed, Jan 08, 2020 at 07:09:07PM +0100, Christian Brauner wrote:
> On Tue, Jan 07, 2020 at 08:32:04AM -0800, Tejun Heo wrote:
> > On Mon, Dec 23, 2019 at 07:15:03AM +0100, Christian Brauner wrote:
> > > +static struct cgroup *cgroup_get_from_file(struct file *f)
> > > +{
> > > +	struct cgroup_subsys_state *css;
> > > +	struct cgroup *cgrp;
> > > +
> > > +	css = css_tryget_online_from_dir(f->f_path.dentry, NULL);
> > > +	if (IS_ERR(css))
> > > +		return ERR_CAST(css);
> > > +
> > > +	cgrp = css->cgroup;
> > > +	if (!cgroup_on_dfl(cgrp)) {
> > > +		cgroup_put(cgrp);
> > > +		return ERR_PTR(-EBADF);
> > > +	}
> > > +
> > > +	return cgrp;
> > > +}
> > 
> > It's minor but can you put this refactoring into a separate patch?
> 
> Yep, will do.
> 
> > 
> > ...
> > > +static int cgroup_css_set_fork(struct task_struct *parent,
> > > +			       struct kernel_clone_args *kargs)
> > > +	__acquires(&cgroup_mutex) __acquires(&cgroup_threadgroup_rwsem)
> > > +{
> > > +	int ret;
> > > +	struct cgroup *dst_cgrp = NULL, *src_cgrp;
> > > +	struct css_set *cset;
> > > +	struct super_block *sb;
> > > +	struct file *f;
> > > +
> > > +	if (kargs->flags & CLONE_INTO_CGROUP) {
> > > +		ret = mutex_lock_killable(&cgroup_mutex);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > 
> > I don't think this is necessary.  cgroup_mutex should always only be
> > held for a finite enough time; otherwise, processes would get stuck on
> > random cgroupfs accesses or even /proc/self/cgroup.
> 
> Ok, so a simple mutex_lock() should suffice then.
> 
> > 
> > ...
> > > +	spin_lock_irq(&css_set_lock);
> > > +	src_cgrp = task_cgroup_from_root(parent, &cgrp_dfl_root);
> > > +	spin_unlock_irq(&css_set_lock);
> > 
> > You can simply do cset->dfl_root here, which is consistent with other
> > code paths which know that they want the dfl cgroup.
> 
> Ah, great!
> 
> > 
> > > +	ret = cgroup_attach_permissions(src_cgrp, dst_cgrp, sb,
> > > +					!!(kargs->flags & CLONE_THREAD));
> > > +	if (ret)
> > > +		goto err;
> > 
> > So, the existing perm check depends on the fact that for the write
> > operation to have started, it already should have passed write perm
> > check on the destination cgroup.procs file.  We're missing that here,
> > so we prolly need to check that explicitly.
> 
> I need to look into this before I can say yay or nay. :)

Could it be that you misread cgroup_attach_permissions()? Because it
does check for write permissions on the destination cgroup.procs file.
That's why I've added the cgroup_get_from_file() helper. :) See:

static int cgroup_attach_permissions(struct cgroup *src_cgrp,
				     struct cgroup *dst_cgrp,
				     struct super_block *sb, bool thread)
{
	int ret = 0;

	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
	if (ret)
		return ret;

	ret = cgroup_migrate_vet_dst(dst_cgrp);
	if (ret)
		return ret;

	if (thread &&
	    !cgroup_same_domain(src_cgrp->dom_cgrp, dst_cgrp->dom_cgrp))
		ret = -EOPNOTSUPP;

	return ret;
}

Maybe I'm misunderstanding though. :)

Thanks!
Christian

^ permalink raw reply

* [PATCH] fscrypt: reserve flags for hardware-wrapped keys feature
From: Eric Biggers @ 2020-01-16 19:20 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Barani Muthukumaran,
	Gaurav Kashyap, Theodore Ts'o, Jaegeuk Kim, linux-api

From: Eric Biggers <ebiggers@google.com>

Reserve flags for the hardware-wrapped keys feature which is being
worked on [1].  FSCRYPT_POLICY_FLAG_HW_WRAPPED_KEY will denote that the
encryption policy needs a hardware-wrapped key to be unlocked.
FSCRYPT_ADD_KEY_FLAG_HW_WRAPPED will denote that the key being added is
a hardware-wrapped key.

This reservation is tentative, and these codepoints may be reused if the
feature is not upstreamed.

[1] https://android-review.googlesource.com/c/kernel/common/+/1200864

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/filesystems/fscrypt.rst | 5 +++--
 fs/crypto/keyring.c                   | 5 ++++-
 fs/crypto/policy.c                    | 4 +++-
 include/uapi/linux/fscrypt.h          | 9 ++++++---
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 9c53336d06a438..4c443d7b1fc6b5 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -639,7 +639,8 @@ follows::
             struct fscrypt_key_specifier key_spec;
             __u32 raw_size;
             __u32 key_id;
-            __u32 __reserved[8];
+            __u32 flags;
+            __u32 __reserved[7];
             __u8 raw[];
     };
 
@@ -658,7 +659,7 @@ follows::
 
     struct fscrypt_provisioning_key_payload {
             __u32 type;
-            __u32 __reserved;
+            __u32 flags;
             __u8 raw[];
     };
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 098ff2e0f0bb41..fc27f5d08d7dbe 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -477,7 +477,7 @@ static int fscrypt_provisioning_key_preparse(struct key_preparsed_payload *prep)
 	    payload->type != FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER)
 		return -EINVAL;
 
-	if (payload->__reserved)
+	if (payload->flags)
 		return -EINVAL;
 
 	prep->payload.data[0] = kmemdup(payload, prep->datalen, GFP_KERNEL);
@@ -606,6 +606,9 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 	if (!valid_key_spec(&arg.key_spec))
 		return -EINVAL;
 
+	if (arg.flags)
+		return -EINVAL;
+
 	if (memchr_inv(arg.__reserved, 0, sizeof(arg.__reserved)))
 		return -EINVAL;
 
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index f1cff83c151acf..36a2bb077b6910 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -139,7 +139,9 @@ static bool fscrypt_supported_v2_policy(const struct fscrypt_policy_v2 *policy,
 		return false;
 	}
 
-	if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID) {
+	if (policy->flags & ~(FSCRYPT_POLICY_FLAGS_PAD_MASK |
+			      FSCRYPT_POLICY_FLAG_DIRECT_KEY |
+			      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64)) {
 		fscrypt_warn(inode, "Unsupported encryption flags (0x%02x)",
 			     policy->flags);
 		return false;
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 0d8a6f47711c32..fad624a4c5feda 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -19,7 +19,8 @@
 #define FSCRYPT_POLICY_FLAGS_PAD_MASK		0x03
 #define FSCRYPT_POLICY_FLAG_DIRECT_KEY		0x04
 #define FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64	0x08
-#define FSCRYPT_POLICY_FLAGS_VALID		0x0F
+#define FSCRYPT_POLICY_FLAG_HW_WRAPPED_KEY	0x10
+#define FSCRYPT_POLICY_FLAGS_VALID		0x1F
 
 /* Encryption algorithms */
 #define FSCRYPT_MODE_AES_256_XTS		1
@@ -116,7 +117,7 @@ struct fscrypt_key_specifier {
  */
 struct fscrypt_provisioning_key_payload {
 	__u32 type;
-	__u32 __reserved;
+	__u32 flags;
 	__u8 raw[];
 };
 
@@ -125,7 +126,9 @@ struct fscrypt_add_key_arg {
 	struct fscrypt_key_specifier key_spec;
 	__u32 raw_size;
 	__u32 key_id;
-	__u32 __reserved[8];
+#define FSCRYPT_ADD_KEY_FLAG_HW_WRAPPED			0x00000001
+	__u32 flags;
+	__u32 __reserved[7];
 	__u8 raw[];
 };
 

base-commit: 2d8f7f119b0b2ce5e7ff0e8024b0763bf42b99c9
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* Re: [PATCH v2 2/3] clone3: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-16 23:57 UTC (permalink / raw)
  To: Michal Koutný
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Ingo Molnar,
	Oleg Nesterov, Johannes Weiner, Li Zefan, Peter Zijlstra,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200108181049.ysmo73celrt4bvo2@wittgenstein>

On Wed, Jan 08, 2020 at 07:10:49PM +0100, Christian Brauner wrote:
> On Wed, Jan 08, 2020 at 05:01:02PM +0100, Michal Koutný wrote:
> > On Mon, Dec 23, 2019 at 07:15:03AM +0100, Christian Brauner <christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote:
> > > This adds support for creating a process in a different cgroup than its
> > > parent.
> > Binding fork and migration together looks useful.
> > 
> > > --- a/kernel/cgroup/cgroup.c
> > > +++ b/kernel/cgroup/cgroup.c
> > > @@ -5882,21 +5882,176 @@ void cgroup_fork(struct task_struct *child)
> > >  	INIT_LIST_HEAD(&child->cg_list);
> > Just a nitpick, I noticed the comment for cgroup_fork should be updated
> > too (generic migration happens in cgroup_post_fork).
> 
> Thanks.
> 
> > 
> > > --- a/kernel/fork.c
> > > +++ b/kernel/fork.c
> > > [...]
> > > @@ -2279,8 +2278,7 @@ static __latent_entropy struct task_struct *copy_process(
> > >  	write_unlock_irq(&tasklist_lock);
> > >  
> > >  	proc_fork_connector(p);
> > > -	cgroup_post_fork(p);
> > > -	cgroup_threadgroup_change_end(current);
> > > +	cgroup_post_fork(current, p, args);
> > I can see that when CLONE_INTO_CGROUP | CLONE_NEWCGROUP is passed, then
> > the child's cgroup NS will be rooted at parent's css set
> > (copy_namespaces precedes cgroup_post_fork).
> > 
> > Wouldn't it make better sense if this flags combination resulted in
> > child's NS rooted in its css set?
> 
> I need to take a closer look but it sounds like we should move the
> copying of the cgroup namespace to a later point; but again I need to
> look into this.

Ok, this is way simpler I think, we just set the root_cset of the new
cgroup namespace to the child's cset in cgroup_post_fork() if
CLONE_NEWCGROUP is requested.

Christian

^ permalink raw reply

* [PATCH v2 0/5] introduce memory hinting API for external process
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api, oleksandr, Suren Baghdasaryan,
	Tim Murray, Daniel Colascione, Sandeep Patil, Sonny Rao,
	Brian Geffon, Michal Hocko, Johannes Weiner, Shakeel Butt,
	John Dias, ktkhai, christian.brauner, sjpark, Minchan Kim

Now, we have MADV_PAGEOUT and MADV_COLD as madvise hinting API. With that,
application could give hints to kernel what memory range are preferred to be
reclaimed. However, in some platform(e.g., Android), the information
required to make the hinting decision is not known to the app.
Instead, it is known to a centralized userspace daemon(e.g., ActivityManagerService),
and that daemon must be able to initiate reclaim on its own without any app
involvement.

To solve the concern, this patch introduces new syscall - process_madvise(2).
Bascially, it's same with madvise(2) syscall but it has some differences.

1. It needs pidfd of target process to provide the hint
2. It supports only MADV_{COLD|PAGEOUT|MERGEABLE|UNMEREABLE} at this moment.
   Other hints in madvise will be opened when there are explicit requests from
   community to prevent unexpected bugs we couldn't support.
3. Only privileged processes can do something for other process's address
   space.

Minchan Kim (3):
  mm: factor out madvise's core functionality
  mm: introduce external memory hinting API
  mm: support both pid and pidfd for process_madvise

Oleksandr Natalenko (2):
  mm/madvise: employ mmget_still_valid for write lock
  mm/madvise: allow KSM hints for remote API

* from v1 - https://lore.kernel.org/linux-mm/20200110213433.94739-1-minchan@kernel.org/
  * fix syscall number - SeongJae
  * use get_pid_task - Kirill
  * extend API to support pid as well as pidfd - Kirill

 arch/alpha/kernel/syscalls/syscall.tbl      |   1 +
 arch/arm/tools/syscall.tbl                  |   1 +
 arch/arm64/include/asm/unistd.h             |   2 +-
 arch/arm64/include/asm/unistd32.h           |   2 +
 arch/ia64/kernel/syscalls/syscall.tbl       |   1 +
 arch/m68k/kernel/syscalls/syscall.tbl       |   1 +
 arch/microblaze/kernel/syscalls/syscall.tbl |   1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |   1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |   1 +
 arch/parisc/kernel/syscalls/syscall.tbl     |   1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |   1 +
 arch/s390/kernel/syscalls/syscall.tbl       |   1 +
 arch/sh/kernel/syscalls/syscall.tbl         |   1 +
 arch/sparc/kernel/syscalls/syscall.tbl      |   1 +
 arch/x86/entry/syscalls/syscall_32.tbl      |   1 +
 arch/x86/entry/syscalls/syscall_64.tbl      |   1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |   1 +
 include/linux/pid.h                         |   1 +
 include/linux/syscalls.h                    |   3 +
 include/uapi/asm-generic/unistd.h           |   5 +-
 kernel/exit.c                               |  17 --
 kernel/pid.c                                |  17 ++
 kernel/sys_ni.c                             |   1 +
 mm/madvise.c                                | 272 ++++++++++++++------
 24 files changed, 233 insertions(+), 102 deletions(-)

-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply

* [PATCH v2 1/5] mm: factor out madvise's core functionality
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api, oleksandr, Suren Baghdasaryan,
	Tim Murray, Daniel Colascione, Sandeep Patil, Sonny Rao,
	Brian Geffon, Michal Hocko, Johannes Weiner, Shakeel Butt,
	John Dias, ktkhai, christian.brauner, sjpark, Minchan Kim
In-Reply-To: <20200116235953.163318-1-minchan@kernel.org>

This patch factor out madvise's core functionality so that upcoming
patch can reuse it without duplication. It shouldn't change any behavior.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/madvise.c | 194 +++++++++++++++++++++++++++++----------------------
 1 file changed, 111 insertions(+), 83 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index bcdb6a042787..0c901de531e4 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -35,6 +35,7 @@
 struct madvise_walk_private {
 	struct mmu_gather *tlb;
 	bool pageout;
+	struct task_struct *task;
 };
 
 /*
@@ -306,12 +307,13 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 	bool pageout = private->pageout;
 	struct mm_struct *mm = tlb->mm;
 	struct vm_area_struct *vma = walk->vma;
+	struct task_struct *task = private->task;
 	pte_t *orig_pte, *pte, ptent;
 	spinlock_t *ptl;
 	struct page *page = NULL;
 	LIST_HEAD(page_list);
 
-	if (fatal_signal_pending(current))
+	if (fatal_signal_pending(task))
 		return -EINTR;
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
@@ -469,12 +471,14 @@ static const struct mm_walk_ops cold_walk_ops = {
 };
 
 static void madvise_cold_page_range(struct mmu_gather *tlb,
+			     struct task_struct *task,
 			     struct vm_area_struct *vma,
 			     unsigned long addr, unsigned long end)
 {
 	struct madvise_walk_private walk_private = {
 		.pageout = false,
 		.tlb = tlb,
+		.task = task,
 	};
 
 	tlb_start_vma(tlb, vma);
@@ -482,7 +486,7 @@ static void madvise_cold_page_range(struct mmu_gather *tlb,
 	tlb_end_vma(tlb, vma);
 }
 
-static long madvise_cold(struct vm_area_struct *vma,
+static long madvise_cold(struct task_struct *task, struct vm_area_struct *vma,
 			struct vm_area_struct **prev,
 			unsigned long start_addr, unsigned long end_addr)
 {
@@ -495,19 +499,21 @@ static long madvise_cold(struct vm_area_struct *vma,
 
 	lru_add_drain();
 	tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
-	madvise_cold_page_range(&tlb, vma, start_addr, end_addr);
+	madvise_cold_page_range(&tlb, task, vma, start_addr, end_addr);
 	tlb_finish_mmu(&tlb, start_addr, end_addr);
 
 	return 0;
 }
 
 static void madvise_pageout_page_range(struct mmu_gather *tlb,
+			     struct task_struct *task,
 			     struct vm_area_struct *vma,
 			     unsigned long addr, unsigned long end)
 {
 	struct madvise_walk_private walk_private = {
 		.pageout = true,
 		.tlb = tlb,
+		.task = task,
 	};
 
 	tlb_start_vma(tlb, vma);
@@ -531,9 +537,9 @@ static inline bool can_do_pageout(struct vm_area_struct *vma)
 		inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
 }
 
-static long madvise_pageout(struct vm_area_struct *vma,
-			struct vm_area_struct **prev,
-			unsigned long start_addr, unsigned long end_addr)
+static long madvise_pageout(struct task_struct *task,
+		struct vm_area_struct *vma, struct vm_area_struct **prev,
+		unsigned long start_addr, unsigned long end_addr)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct mmu_gather tlb;
@@ -547,7 +553,7 @@ static long madvise_pageout(struct vm_area_struct *vma,
 
 	lru_add_drain();
 	tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
-	madvise_pageout_page_range(&tlb, vma, start_addr, end_addr);
+	madvise_pageout_page_range(&tlb, task, vma, start_addr, end_addr);
 	tlb_finish_mmu(&tlb, start_addr, end_addr);
 
 	return 0;
@@ -751,7 +757,8 @@ static long madvise_dontneed_single_vma(struct vm_area_struct *vma,
 	return 0;
 }
 
-static long madvise_dontneed_free(struct vm_area_struct *vma,
+static long madvise_dontneed_free(struct mm_struct *mm,
+				  struct vm_area_struct *vma,
 				  struct vm_area_struct **prev,
 				  unsigned long start, unsigned long end,
 				  int behavior)
@@ -763,8 +770,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
 	if (!userfaultfd_remove(vma, start, end)) {
 		*prev = NULL; /* mmap_sem has been dropped, prev is stale */
 
-		down_read(&current->mm->mmap_sem);
-		vma = find_vma(current->mm, start);
+		down_read(&mm->mmap_sem);
+		vma = find_vma(mm, start);
 		if (!vma)
 			return -ENOMEM;
 		if (start < vma->vm_start) {
@@ -811,7 +818,8 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
  * Application wants to free up the pages and associated backing store.
  * This is effectively punching a hole into the middle of a file.
  */
-static long madvise_remove(struct vm_area_struct *vma,
+static long madvise_remove(struct mm_struct *mm,
+				struct vm_area_struct *vma,
 				struct vm_area_struct **prev,
 				unsigned long start, unsigned long end)
 {
@@ -845,13 +853,13 @@ static long madvise_remove(struct vm_area_struct *vma,
 	get_file(f);
 	if (userfaultfd_remove(vma, start, end)) {
 		/* mmap_sem was not released by userfaultfd_remove() */
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	}
 	error = vfs_fallocate(f,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				offset, end - start);
 	fput(f);
-	down_read(&current->mm->mmap_sem);
+	down_read(&mm->mmap_sem);
 	return error;
 }
 
@@ -925,21 +933,23 @@ static int madvise_inject_error(int behavior,
 #endif
 
 static long
-madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
+madvise_vma(struct task_struct *task, struct mm_struct *mm,
+		struct vm_area_struct *vma, struct vm_area_struct **prev,
 		unsigned long start, unsigned long end, int behavior)
 {
 	switch (behavior) {
 	case MADV_REMOVE:
-		return madvise_remove(vma, prev, start, end);
+		return madvise_remove(mm, vma, prev, start, end);
 	case MADV_WILLNEED:
 		return madvise_willneed(vma, prev, start, end);
 	case MADV_COLD:
-		return madvise_cold(vma, prev, start, end);
+		return madvise_cold(task, vma, prev, start, end);
 	case MADV_PAGEOUT:
-		return madvise_pageout(vma, prev, start, end);
+		return madvise_pageout(task, vma, prev, start, end);
 	case MADV_FREE:
 	case MADV_DONTNEED:
-		return madvise_dontneed_free(vma, prev, start, end, behavior);
+		return madvise_dontneed_free(mm, vma, prev, start,
+						end, behavior);
 	default:
 		return madvise_behavior(vma, prev, start, end, behavior);
 	}
@@ -984,67 +994,19 @@ madvise_behavior_valid(int behavior)
 }
 
 /*
- * The madvise(2) system call.
+ * madvise_common - request behavior hint to address range of the target process
  *
- * Applications can use madvise() to advise the kernel how it should
- * handle paging I/O in this VM area.  The idea is to help the kernel
- * use appropriate read-ahead and caching techniques.  The information
- * provided is advisory only, and can be safely disregarded by the
- * kernel without affecting the correct operation of the application.
+ * @task: task_struct got behavior hint, not giving the hint
+ * @mm: mm_struct got behavior hint, not giving the hint
+ * @start: base address of the hinted range
+ * @len_in: length of the hinted range
+ * @behavior: requested hint
  *
- * behavior values:
- *  MADV_NORMAL - the default behavior is to read clusters.  This
- *		results in some read-ahead and read-behind.
- *  MADV_RANDOM - the system should read the minimum amount of data
- *		on any access, since it is unlikely that the appli-
- *		cation will need more than what it asks for.
- *  MADV_SEQUENTIAL - pages in the given range will probably be accessed
- *		once, so they can be aggressively read ahead, and
- *		can be freed soon after they are accessed.
- *  MADV_WILLNEED - the application is notifying the system to read
- *		some pages ahead.
- *  MADV_DONTNEED - the application is finished with the given range,
- *		so the kernel can free resources associated with it.
- *  MADV_FREE - the application marks pages in the given range as lazy free,
- *		where actual purges are postponed until memory pressure happens.
- *  MADV_REMOVE - the application wants to free up the given range of
- *		pages and associated backing store.
- *  MADV_DONTFORK - omit this area from child's address space when forking:
- *		typically, to avoid COWing pages pinned by get_user_pages().
- *  MADV_DOFORK - cancel MADV_DONTFORK: no longer omit this area when forking.
- *  MADV_WIPEONFORK - present the child process with zero-filled memory in this
- *              range after a fork.
- *  MADV_KEEPONFORK - undo the effect of MADV_WIPEONFORK
- *  MADV_HWPOISON - trigger memory error handler as if the given memory range
- *		were corrupted by unrecoverable hardware memory failure.
- *  MADV_SOFT_OFFLINE - try to soft-offline the given range of memory.
- *  MADV_MERGEABLE - the application recommends that KSM try to merge pages in
- *		this area with pages of identical content from other such areas.
- *  MADV_UNMERGEABLE- cancel MADV_MERGEABLE: no longer merge pages with others.
- *  MADV_HUGEPAGE - the application wants to back the given range by transparent
- *		huge pages in the future. Existing pages might be coalesced and
- *		new pages might be allocated as THP.
- *  MADV_NOHUGEPAGE - mark the given range as not worth being backed by
- *		transparent huge pages so the existing pages will not be
- *		coalesced into THP and new pages will not be allocated as THP.
- *  MADV_DONTDUMP - the application wants to prevent pages in the given range
- *		from being included in its core dump.
- *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
- *
- * return values:
- *  zero    - success
- *  -EINVAL - start + len < 0, start is not page-aligned,
- *		"behavior" is not a valid value, or application
- *		is attempting to release locked or shared pages,
- *		or the specified address range includes file, Huge TLB,
- *		MAP_SHARED or VMPFNMAP range.
- *  -ENOMEM - addresses in the specified range are not currently
- *		mapped, or are outside the AS of the process.
- *  -EIO    - an I/O error occurred while paging in data.
- *  -EBADF  - map exists, but area maps something that isn't a file.
- *  -EAGAIN - a kernel resource was temporarily unavailable.
+ * @task could be a zombie leader if it calls sys_exit so accessing mm_struct
+ * via task->mm is prohibited. Please use @mm instead of task->mm.
  */
-SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
+static int madvise_common(struct task_struct *task, struct mm_struct *mm,
+			unsigned long start, size_t len_in, int behavior)
 {
 	unsigned long end, tmp;
 	struct vm_area_struct *vma, *prev;
@@ -1082,10 +1044,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 
 	write = madvise_need_mmap_write(behavior);
 	if (write) {
-		if (down_write_killable(&current->mm->mmap_sem))
+		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
 	} else {
-		down_read(&current->mm->mmap_sem);
+		down_read(&mm->mmap_sem);
 	}
 
 	/*
@@ -1093,7 +1055,7 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 	 * ranges, just ignore them, but return -ENOMEM at the end.
 	 * - different from the way of handling in mlock etc.
 	 */
-	vma = find_vma_prev(current->mm, start, &prev);
+	vma = find_vma_prev(mm, start, &prev);
 	if (vma && start > vma->vm_start)
 		prev = vma;
 
@@ -1118,7 +1080,7 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 			tmp = end;
 
 		/* Here vma->vm_start <= start < tmp <= (end|vma->vm_end). */
-		error = madvise_vma(vma, &prev, start, tmp, behavior);
+		error = madvise_vma(task, mm, vma, &prev, start, tmp, behavior);
 		if (error)
 			goto out;
 		start = tmp;
@@ -1130,14 +1092,80 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 		if (prev)
 			vma = prev->vm_next;
 		else	/* madvise_remove dropped mmap_sem */
-			vma = find_vma(current->mm, start);
+			vma = find_vma(mm, start);
 	}
 out:
 	blk_finish_plug(&plug);
 	if (write)
-		up_write(&current->mm->mmap_sem);
+		up_write(&mm->mmap_sem);
 	else
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 
 	return error;
 }
+
+/*
+ * The madvise(2) system call.
+ *
+ * Applications can use madvise() to advise the kernel how it should
+ * handle paging I/O in this VM area.  The idea is to help the kernel
+ * use appropriate read-ahead and caching techniques.  The information
+ * provided is advisory only, and can be safely disregarded by the
+ * kernel without affecting the correct operation of the application.
+ *
+ * behavior values:
+ *  MADV_NORMAL - the default behavior is to read clusters.  This
+ *		results in some read-ahead and read-behind.
+ *  MADV_RANDOM - the system should read the minimum amount of data
+ *		on any access, since it is unlikely that the appli-
+ *		cation will need more than what it asks for.
+ *  MADV_SEQUENTIAL - pages in the given range will probably be accessed
+ *		once, so they can be aggressively read ahead, and
+ *		can be freed soon after they are accessed.
+ *  MADV_WILLNEED - the application is notifying the system to read
+ *		some pages ahead.
+ *  MADV_DONTNEED - the application is finished with the given range,
+ *		so the kernel can free resources associated with it.
+ *  MADV_FREE - the application marks pages in the given range as lazy free,
+ *		where actual purges are postponed until memory pressure happens.
+ *  MADV_REMOVE - the application wants to free up the given range of
+ *		pages and associated backing store.
+ *  MADV_DONTFORK - omit this area from child's address space when forking:
+ *		typically, to avoid COWing pages pinned by get_user_pages().
+ *  MADV_DOFORK - cancel MADV_DONTFORK: no longer omit this area when forking.
+ *  MADV_WIPEONFORK - present the child process with zero-filled memory in this
+ *              range after a fork.
+ *  MADV_KEEPONFORK - undo the effect of MADV_WIPEONFORK
+ *  MADV_HWPOISON - trigger memory error handler as if the given memory range
+ *		were corrupted by unrecoverable hardware memory failure.
+ *  MADV_SOFT_OFFLINE - try to soft-offline the given range of memory.
+ *  MADV_MERGEABLE - the application recommends that KSM try to merge pages in
+ *		this area with pages of identical content from other such areas.
+ *  MADV_UNMERGEABLE- cancel MADV_MERGEABLE: no longer merge pages with others.
+ *  MADV_HUGEPAGE - the application wants to back the given range by transparent
+ *		huge pages in the future. Existing pages might be coalesced and
+ *		new pages might be allocated as THP.
+ *  MADV_NOHUGEPAGE - mark the given range as not worth being backed by
+ *		transparent huge pages so the existing pages will not be
+ *		coalesced into THP and new pages will not be allocated as THP.
+ *  MADV_DONTDUMP - the application wants to prevent pages in the given range
+ *		from being included in its core dump.
+ *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
+ *
+ * return values:
+ *  zero    - success
+ *  -EINVAL - start + len < 0, start is not page-aligned,
+ *		"behavior" is not a valid value, or application
+ *		is attempting to release locked or shared pages,
+ *		or the specified address range includes file, Huge TLB,
+ *		MAP_SHARED or VMPFNMAP range.
+ *  -ENOMEM - addresses in the specified range are not currently
+ *		mapped, or are outside the AS of the process.
+ *  -EIO    - an I/O error occurred while paging in data.
+ *  -EBADF  - map exists, but area maps something that isn't a file.
+ *  -EAGAIN - a kernel resource was temporarily unavailable.
+ */
+SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
+{
+	return madvise_common(current, current->mm, start, len_in, behavior);
+}
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* [PATCH v2 2/5] mm: introduce external memory hinting API
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api-u79uwXL29TY76Z2rM5mHXA,
	oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
	Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
	Michal Hocko, Johannes Weiner, Shakeel Butt, John Dias,
	ktkhai-5HdwGun5lf+gSpxsJD1C4w,
	christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
	sjpark-ebkRAfMGSJGzQB+pC5nmwQ, Minchan Kim
In-Reply-To: <20200116235953.163318-1-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

There is usecase that System Management Software(SMS) want to give
a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
in the case of Android, it is the ActivityManagerService.

It's similar in spirit to madvise(MADV_WONTNEED), but the information
required to make the reclaim decision is not known to the app. Instead,
it is known to the centralized userspace daemon(ActivityManagerService),
and that daemon must be able to initiate reclaim on its own without
any app involvement.

To solve the issue, this patch introduces new syscall process_madvise(2).
It uses pidfd of an external processs to give the hint.

 int process_madvise(int pidfd, void *addr, size_t length, int advise,
			unsigned long flag);

Since it could affect other process's address range, only privileged
process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
gives it the right to ptrace the process could use it successfully.
The flag argument is reserved for future use if we need to extend the
API.

I think supporting all hints madvise has/will supported/support to
process_madvise is rather risky. Because we are not sure all hints make
sense from external process and implementation for the hint may rely on
the caller being in the current context so it could be error-prone.
Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.

If someone want to add other hints, we could hear hear the usecase and
review it for each hint. It's more safe for maintainace rather than
introducing a buggy syscall but hard to fix it later.

Signed-off-by: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 arch/alpha/kernel/syscalls/syscall.tbl      |  1 +
 arch/arm/tools/syscall.tbl                  |  1 +
 arch/arm64/include/asm/unistd.h             |  2 +-
 arch/arm64/include/asm/unistd32.h           |  2 +
 arch/ia64/kernel/syscalls/syscall.tbl       |  1 +
 arch/m68k/kernel/syscalls/syscall.tbl       |  1 +
 arch/microblaze/kernel/syscalls/syscall.tbl |  1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |  1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |  1 +
 arch/parisc/kernel/syscalls/syscall.tbl     |  1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |  1 +
 arch/s390/kernel/syscalls/syscall.tbl       |  1 +
 arch/sh/kernel/syscalls/syscall.tbl         |  1 +
 arch/sparc/kernel/syscalls/syscall.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_32.tbl      |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl      |  1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |  1 +
 include/linux/syscalls.h                    |  2 +
 include/uapi/asm-generic/unistd.h           |  5 +-
 kernel/sys_ni.c                             |  1 +
 mm/madvise.c                                | 63 +++++++++++++++++++++
 21 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index e56950f23b49..776c61803315 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -477,3 +477,4 @@
 # 545 reserved for clone3
 546	common	watch_devices			sys_watch_devices
 547	common	openat2				sys_openat2
+548	common	process_madvise			sys_process_madvise
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 7fb2f4d59210..a43381542276 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -451,3 +451,4 @@
 435	common	clone3				sys_clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 8aa00ccb0b96..b722e47377a5 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -38,7 +38,7 @@
 #define __ARM_NR_compat_set_tls		(__ARM_NR_COMPAT_BASE + 5)
 #define __ARM_NR_COMPAT_END		(__ARM_NR_COMPAT_BASE + 0x800)
 
-#define __NR_compat_syscalls		438
+#define __NR_compat_syscalls		439
 #endif
 
 #define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 31f0ce25719e..e3643d7fecc3 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -883,6 +883,8 @@ __SYSCALL(__NR_clone3, sys_clone3)
 __SYSCALL(__NR_watch_devices, sys_watch_devices)
 #define __NR_openat2 437
 __SYSCALL(__NR_openat2, sys_openat2)
+#define __NR_process_madvise 438
+__SYSCALL(__NR_process_madvise, process_madvise)
 
 /*
  * Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index b9aa59931905..c156abc9a298 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -358,3 +358,4 @@
 # 435 reserved for clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 868c1ef89d35..5b6034b6650f 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -437,3 +437,4 @@
 # 435 reserved for clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 544b4cef18b3..4bef584af09c 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -443,3 +443,4 @@
 435	common	clone3				sys_clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 05e8aee5dae7..7061b2103438 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -376,3 +376,4 @@
 435	n32	clone3				__sys_clone3
 436	n32	watch_devices			sys_watch_devices
 437	n32	openat2				sys_openat2
+438	n32	process_madivse			sys_process_madvise
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 24d6c01328fb..84042d57fbfb 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -352,3 +352,4 @@
 435	n64	clone3				__sys_clone3
 436	n64	watch_devices			sys_watch_devices
 437	n64	openat2				sys_openat2
+438	n64	process_madvise			sys_process_madvise
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 4b5f77a4e1a2..5bfd359c7e6f 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -435,3 +435,4 @@
 435	common	clone3				sys_clone3_wrapper
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 9716dc85a517..ffa0e679aca0 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -519,3 +519,4 @@
 435	nospu	clone3				ppc_clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 7da330f8b03e..c301717216ca 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 435  common	clone3			sys_clone3			sys_clone3
 436  common	watch_devices		sys_watch_devices		sys_watch_devices
 437  common	openat2			sys_openat2			sys_openat2
+438  common	process_madvise		sys_process_madvise		sys_process_madvise
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index bb7e68e25337..b8f15701f69f 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 # 435 reserved for clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 646a1fad7218..7ea95f37b222 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -483,3 +483,4 @@
 # 435 reserved for clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2			sys_openat2
+438	common	process_madvise		sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 57c53acee290..76a2c266fe7e 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -442,3 +442,4 @@
 435	i386	clone3			sys_clone3			__ia32_sys_clone3
 436	i386	watch_devices		sys_watch_devices		__ia32_sys_watch_devices
 437	i386	openat2			sys_openat2			__ia32_sys_openat2
+438	i386	process_madvise		sys_process_madvise		__ia32_sys_process_madvise
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 1dd8d21f6500..b697cd8620cb 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -359,6 +359,7 @@
 435	common	clone3			__x64_sys_clone3/ptregs
 436	common	watch_devices		__x64_sys_watch_devices
 437	common	openat2			__x64_sys_openat2
+438	common	process_madvise		__x64_sys_process_madvise
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 0f48ab7bd75b..2e9813ecfd7d 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -408,3 +408,4 @@
 435	common	clone3				sys_clone3
 436	common	watch_devices			sys_watch_devices
 437	common	openat2				sys_openat2
+438	common	process_madvise			sys_process_madvise
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 433c8c85636e..1b58a11ff49f 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -877,6 +877,8 @@ asmlinkage long sys_munlockall(void);
 asmlinkage long sys_mincore(unsigned long start, size_t len,
 				unsigned char __user * vec);
 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
+asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
+			size_t len, int behavior, unsigned long flags);
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 			unsigned long prot, unsigned long pgoff,
 			unsigned long flags);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 33f3856a9c3c..4a49fbaea013 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -856,8 +856,11 @@ __SYSCALL(__NR_watch_devices, sys_watch_devices)
 #define __NR_openat2 437
 __SYSCALL(__NR_openat2, sys_openat2)
 
+#define __NR_process_madvise 438
+__SYSCALL(__NR_process_madvise, sys_process_madvise)
+
 #undef __NR_syscalls
-#define __NR_syscalls 438
+#define __NR_syscalls 439
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 0e9b275260f8..10ce5eac8b4b 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -281,6 +281,7 @@ COND_SYSCALL(mlockall);
 COND_SYSCALL(munlockall);
 COND_SYSCALL(mincore);
 COND_SYSCALL(madvise);
+COND_SYSCALL(process_madvise);
 COND_SYSCALL(remap_file_pages);
 COND_SYSCALL(mbind);
 COND_SYSCALL_COMPAT(mbind);
diff --git a/mm/madvise.c b/mm/madvise.c
index 0c901de531e4..59b5cc99ef61 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -17,6 +17,7 @@
 #include <linux/falloc.h>
 #include <linux/fadvise.h>
 #include <linux/sched.h>
+#include <linux/sched/mm.h>
 #include <linux/ksm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
@@ -993,6 +994,18 @@ madvise_behavior_valid(int behavior)
 	}
 }
 
+static bool
+process_madvise_behavior_valid(int behavior)
+{
+	switch (behavior) {
+	case MADV_COLD:
+	case MADV_PAGEOUT:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * madvise_common - request behavior hint to address range of the target process
  *
@@ -1151,6 +1164,10 @@ static int madvise_common(struct task_struct *task, struct mm_struct *mm,
  *  MADV_DONTDUMP - the application wants to prevent pages in the given range
  *		from being included in its core dump.
  *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
+ *  MADV_COLD - the application uses the memory less so the kernel can deactivate
+ *  		the memory to evict them quickly when the memory pressure happen.
+ *  MADV_PAGEOUT - the application uses the memroy very rarely so kernel can
+ *  		page out the memory instantly.
  *
  * return values:
  *  zero    - success
@@ -1169,3 +1186,49 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 {
 	return madvise_common(current, current->mm, start, len_in, behavior);
 }
+
+SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
+		size_t, len_in, int, behavior, unsigned long, flags)
+{
+	int ret;
+	struct fd f;
+	struct pid *pid;
+	struct task_struct *task;
+	struct mm_struct *mm;
+
+	if (flags != 0)
+		return -EINVAL;
+
+	if (!process_madvise_behavior_valid(behavior))
+		return -EINVAL;
+
+	f = fdget(pidfd);
+	if (!f.file)
+		return -EBADF;
+
+	pid = pidfd_pid(f.file);
+	if (IS_ERR(pid)) {
+		ret = PTR_ERR(pid);
+		goto fdput;
+	}
+
+	task = get_pid_task(pid, PIDTYPE_PID);
+	if (!task) {
+		ret = -ESRCH;
+		goto fdput;
+	}
+
+	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
+	if (IS_ERR_OR_NULL(mm)) {
+		ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
+		goto release_task;
+	}
+
+	ret = madvise_common(task, mm, start, len_in, behavior);
+	mmput(mm);
+release_task:
+	put_task_struct(task);
+fdput:
+	fdput(f);
+	return ret;
+}
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* [PATCH v2 3/5] mm/madvise: employ mmget_still_valid for write lock
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api, oleksandr, Suren Baghdasaryan,
	Tim Murray, Daniel Colascione, Sandeep Patil, Sonny Rao,
	Brian Geffon, Michal Hocko, Johannes Weiner, Shakeel Butt,
	John Dias, ktkhai, christian.brauner, sjpark, Minchan Kim,
	Minchan Kim
In-Reply-To: <20200116235953.163318-1-minchan@kernel.org>

From: Oleksandr Natalenko <oleksandr@redhat.com>

Do the very same trick as we already do since 04f5866e41fb. KSM hints
will require locking mmap_sem for write since they modify vm_flags, so
for remote KSM hinting this additional check is needed.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
Signed-off-by: Minchan Kim <minchan@google.com>
---
 mm/madvise.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index 59b5cc99ef61..84cffd0900f1 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1059,6 +1059,8 @@ static int madvise_common(struct task_struct *task, struct mm_struct *mm,
 	if (write) {
 		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
+		if (current->mm != mm && !mmget_still_valid(mm))
+			goto skip_mm;
 	} else {
 		down_read(&mm->mmap_sem);
 	}
@@ -1109,6 +1111,7 @@ static int madvise_common(struct task_struct *task, struct mm_struct *mm,
 	}
 out:
 	blk_finish_plug(&plug);
+skip_mm:
 	if (write)
 		up_write(&mm->mmap_sem);
 	else
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* [PATCH v2 4/5] mm/madvise: allow KSM hints for remote API
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api, oleksandr, Suren Baghdasaryan,
	Tim Murray, Daniel Colascione, Sandeep Patil, Sonny Rao,
	Brian Geffon, Michal Hocko, Johannes Weiner, Shakeel Butt,
	John Dias, ktkhai, christian.brauner, sjpark, Minchan Kim,
	Minchan Kim
In-Reply-To: <20200116235953.163318-1-minchan@kernel.org>

From: Oleksandr Natalenko <oleksandr@redhat.com>

It all began with the fact that KSM works only on memory that is marked
by madvise(). And the only way to get around that is to either:

  * use LD_PRELOAD; or
  * patch the kernel with something like UKSM or PKSM.

(i skip ptrace can of worms here intentionally)

To overcome this restriction, lets employ a new remote madvise API. This
can be used by some small userspace helper daemon that will do auto-KSM
job for us.

I think of two major consumers of remote KSM hints:

  * hosts, that run containers, especially similar ones and especially in
    a trusted environment, sharing the same runtime like Node.js;

  * heavy applications, that can be run in multiple instances, not
    limited to opensource ones like Firefox, but also those that cannot be
    modified since they are binary-only and, maybe, statically linked.

Speaking of statistics, more numbers can be found in the very first
submission, that is related to this one [1]. For my current setup with
two Firefox instances I get 100 to 200 MiB saved for the second instance
depending on the amount of tabs.

1 FF instance with 15 tabs:

   $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
   410

2 FF instances, second one has 12 tabs (all the tabs are different):

   $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
   592

At the very moment I do not have specific numbers for containerised
workload, but those should be comparable in case the containers share
similar/same runtime.

[1] https://lore.kernel.org/patchwork/patch/1012142/

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
Signed-off-by: Minchan Kim <minchan@google.com>
---
 mm/madvise.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index 84cffd0900f1..89557998d287 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1000,6 +1000,8 @@ process_madvise_behavior_valid(int behavior)
 	switch (behavior) {
 	case MADV_COLD:
 	case MADV_PAGEOUT:
+	case MADV_MERGEABLE:
+	case MADV_UNMERGEABLE:
 		return true;
 	default:
 		return false;
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* [PATCH v2 5/5] mm: support both pid and pidfd for process_madvise
From: Minchan Kim @ 2020-01-16 23:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: LKML, linux-mm, linux-api, oleksandr, Suren Baghdasaryan,
	Tim Murray, Daniel Colascione, Sandeep Patil, Sonny Rao,
	Brian Geffon, Michal Hocko, Johannes Weiner, Shakeel Butt,
	John Dias, ktkhai, christian.brauner, sjpark, Minchan Kim
In-Reply-To: <20200116235953.163318-1-minchan@kernel.org>

There is a demand[1] to support pid as well pidfd for process_madvise
to reduce unncessary syscall to get pidfd if the user has control of
the targer process(ie, they could gaurantee the process is not gone
or pid is not reused. Or, it might be okay to give a hint to wrong
process).

This patch aims for supporting both options like waitid(2). So, the
syscall is currently,

	int process_madvise(int which, pid_t pid, void *addr,
		size_t length, int advise, unsigned long flag);

@which is actually idtype_t for userspace libray and currently,
it supports P_PID and P_PIDFD.

[1]  https://lore.kernel.org/linux-mm/9d849087-3359-c4ab-fbec-859e8186c509@virtuozzo.com/
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/pid.h      |  1 +
 include/linux/syscalls.h |  3 ++-
 kernel/exit.c            | 17 -----------------
 kernel/pid.c             | 17 +++++++++++++++++
 mm/madvise.c             | 34 ++++++++++++++++++++++------------
 5 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/include/linux/pid.h b/include/linux/pid.h
index 998ae7d24450..023d9c3a8edc 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -75,6 +75,7 @@ extern const struct file_operations pidfd_fops;
 struct file;
 
 extern struct pid *pidfd_pid(const struct file *file);
+extern struct pid *pidfd_get_pid(unsigned int fd);
 
 static inline struct pid *get_pid(struct pid *pid)
 {
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 1b58a11ff49f..27060e59db37 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -877,7 +877,8 @@ asmlinkage long sys_munlockall(void);
 asmlinkage long sys_mincore(unsigned long start, size_t len,
 				unsigned char __user * vec);
 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
-asmlinkage long sys_process_madvise(int pidfd, unsigned long start,
+
+asmlinkage long sys_process_madvise(int which, pid_t pid, unsigned long start,
 			size_t len, int behavior, unsigned long flags);
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 			unsigned long prot, unsigned long pgoff,
diff --git a/kernel/exit.c b/kernel/exit.c
index bcbd59888e67..7698843b1411 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1466,23 +1466,6 @@ static long do_wait(struct wait_opts *wo)
 	return retval;
 }
 
-static struct pid *pidfd_get_pid(unsigned int fd)
-{
-	struct fd f;
-	struct pid *pid;
-
-	f = fdget(fd);
-	if (!f.file)
-		return ERR_PTR(-EBADF);
-
-	pid = pidfd_pid(f.file);
-	if (!IS_ERR(pid))
-		get_pid(pid);
-
-	fdput(f);
-	return pid;
-}
-
 static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop,
 			  int options, struct rusage *ru)
 {
diff --git a/kernel/pid.c b/kernel/pid.c
index 2278e249141d..a41a89d5dad2 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -496,6 +496,23 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
 	return idr_get_next(&ns->idr, &nr);
 }
 
+struct pid *pidfd_get_pid(unsigned int fd)
+{
+	struct fd f;
+	struct pid *pid;
+
+	f = fdget(fd);
+	if (!f.file)
+		return ERR_PTR(-EBADF);
+
+	pid = pidfd_pid(f.file);
+	if (!IS_ERR(pid))
+		get_pid(pid);
+
+	fdput(f);
+	return pid;
+}
+
 /**
  * pidfd_create() - Create a new pid file descriptor.
  *
diff --git a/mm/madvise.c b/mm/madvise.c
index 89557998d287..2ac62716e5b8 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1192,11 +1192,10 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 	return madvise_common(current, current->mm, start, len_in, behavior);
 }
 
-SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
+SYSCALL_DEFINE6(process_madvise, int, which, pid_t, upid, unsigned long, start,
 		size_t, len_in, int, behavior, unsigned long, flags)
 {
 	int ret;
-	struct fd f;
 	struct pid *pid;
 	struct task_struct *task;
 	struct mm_struct *mm;
@@ -1207,20 +1206,31 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
 	if (!process_madvise_behavior_valid(behavior))
 		return -EINVAL;
 
-	f = fdget(pidfd);
-	if (!f.file)
-		return -EBADF;
+	switch (which) {
+	case P_PID:
+		if (upid <= 0)
+			return -EINVAL;
+
+		pid = find_get_pid(upid);
+		if (!pid)
+			return -ESRCH;
+		break;
+	case P_PIDFD:
+		if (upid < 0)
+			return -EINVAL;
 
-	pid = pidfd_pid(f.file);
-	if (IS_ERR(pid)) {
-		ret = PTR_ERR(pid);
-		goto fdput;
+		pid = pidfd_get_pid(upid);
+		if (IS_ERR(pid))
+			return PTR_ERR(pid);
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	task = get_pid_task(pid, PIDTYPE_PID);
 	if (!task) {
 		ret = -ESRCH;
-		goto fdput;
+		goto put_pid;
 	}
 
 	mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
@@ -1233,7 +1243,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
 	mmput(mm);
 release_task:
 	put_task_struct(task);
-fdput:
-	fdput(f);
+put_pid:
+	put_pid(pid);
 	return ret;
 }
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply related

* [PATCH v3 0/5] clone3 & cgroups: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner

Hey Tejun,

This is v3 of the promised series to enable spawning processes into a
target cgroup different from the parent's cgroup.

/* v1 */
Link: https://lore.kernel.org/r/20191218173516.7875-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org

/* v2 */
Link: https://lore.kernel.org/r/20191223061504.28716-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
Rework locking and remove unneeded helper functions. Please see
individual patch changelogs for details.
With this I've been able to run the cgroup selftests and stress tests in
loops for a long time without any regressions or deadlocks; lockdep and
kasan did not complain either.

/* v3 */
Split preliminary work into separate patches.
See changelog of individual commits.

With this cgroup migration will be a lot easier, and accounting will be
more exact. It also allows for nice features such as creating a frozen
process by spawning it into a frozen cgroup.
The code simplifies container creation and exec logic quite a bit as
well.

I've tried to contain all core changes for this features in
kernel/cgroup/* to avoid exposing cgroup internals. This has mostly
worked.
When a new process is supposed to be spawned in a cgroup different from
the parent's then we briefly acquire the cgroup mutex right before
fork()'s point of no return and drop it once the child process has been
attached to the tasklist and to its css_set. This is done to ensure that
the cgroup isn't removed behind our back. The cgroup mutex is _only_
held in this case; the usual case, where the child is created in the
same cgroup as the parent does not acquire it since the cgroup can't be
removed.

The series already comes with proper testing. Once we've decided that
this approach is good I'll expand the test-suite even more.

The branch can be found in the following locations:
[1]: kernel.org: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clone_into_cgroup
[2]: github.com: https://github.com/brauner/linux/tree/clone_into_cgroup
[3]: gitlab.com: https://gitlab.com/brauner/linux/commits/clone_into_cgroup

Thanks!
Christian

Christian Brauner (5):
  cgroup: unify attach permission checking
  cgroup: add cgroup_get_from_file() helper
  cgroup: refactor fork helpers
  clone3: allow spawning processes into cgroups
  selftests/cgroup: add tests for cloning into cgroups

 include/linux/cgroup-defs.h                   |   6 +-
 include/linux/cgroup.h                        |  26 +-
 include/linux/sched/task.h                    |   4 +
 include/uapi/linux/sched.h                    |   5 +
 kernel/cgroup/cgroup.c                        | 277 ++++++++++++++----
 kernel/cgroup/pids.c                          |  16 +-
 kernel/fork.c                                 |  19 +-
 tools/testing/selftests/cgroup/Makefile       |   6 +-
 tools/testing/selftests/cgroup/cgroup_util.c  | 126 ++++++++
 tools/testing/selftests/cgroup/cgroup_util.h  |   4 +
 tools/testing/selftests/cgroup/test_core.c    |  64 ++++
 .../selftests/clone3/clone3_selftests.h       |  19 +-
 12 files changed, 495 insertions(+), 77 deletions(-)


base-commit: b3a987b0264d3ddbb24293ebff10eddfc472f653
-- 
2.25.0

^ permalink raw reply

* [PATCH v3 1/5] cgroup: unify attach permission checking
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api, linux-kernel, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner, Li Zefan, Johannes Weiner,
	cgroups
In-Reply-To: <20200117002143.15559-1-christian.brauner@ubuntu.com>

The core codepaths to check whether a process can be attached to a
cgroup are the same for threads and thread-group leaders. Only a small
piece of code verifying that source and destination cgroup are in the
same domain differentiates the thread permission checking from
thread-group leader permission checking.
Since cgroup_migrate_vet_dst() only matters cgroup2 - it is a noop on
cgroup1 - we can move it out of cgroup_attach_task().
All checks can now be consolidated into a new helper
cgroup_attach_permissions() callable from both cgroup_procs_write() and
cgroup_threads_write().

Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v1 */
Link: https://lore.kernel.org/r/20191218173516.7875-2-christian.brauner@ubuntu.com

/* v2 */
Link: https://lore.kernel.org/r/20191223061504.28716-2-christian.brauner@ubuntu.com
- Christian Brauner <christian.brauner@ubuntu.com>:
  - Fix return value of cgroup_attach_permissions. It used to return 0
    when it should've returned -EOPNOTSUPP.
  - Fix call to cgroup_attach_permissions() in cgroup_procs_write(). It
    accidently specified that a thread was moved causing an additional
    check for domain-group equality to be executed that is not needed.

/* v3 */
unchanged
---
 kernel/cgroup/cgroup.c | 46 +++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 735af8f15f95..ad1f9fea5c14 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2719,11 +2719,7 @@ int cgroup_attach_task(struct cgroup *dst_cgrp, struct task_struct *leader,
 {
 	DEFINE_CGROUP_MGCTX(mgctx);
 	struct task_struct *task;
-	int ret;
-
-	ret = cgroup_migrate_vet_dst(dst_cgrp);
-	if (ret)
-		return ret;
+	int ret = 0;
 
 	/* look up all src csets */
 	spin_lock_irq(&css_set_lock);
@@ -4690,6 +4686,33 @@ static int cgroup_procs_write_permission(struct cgroup *src_cgrp,
 	return 0;
 }
 
+static inline bool cgroup_same_domain(const struct cgroup *src_cgrp,
+				      const struct cgroup *dst_cgrp)
+{
+	return src_cgrp->dom_cgrp == dst_cgrp->dom_cgrp;
+}
+
+static int cgroup_attach_permissions(struct cgroup *src_cgrp,
+				     struct cgroup *dst_cgrp,
+				     struct super_block *sb, bool thread)
+{
+	int ret = 0;
+
+	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
+	if (ret)
+		return ret;
+
+	ret = cgroup_migrate_vet_dst(dst_cgrp);
+	if (ret)
+		return ret;
+
+	if (thread &&
+	    !cgroup_same_domain(src_cgrp->dom_cgrp, dst_cgrp->dom_cgrp))
+		ret = -EOPNOTSUPP;
+
+	return ret;
+}
+
 static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
 				  char *buf, size_t nbytes, loff_t off)
 {
@@ -4712,8 +4735,8 @@ static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
 	src_cgrp = task_cgroup_from_root(task, &cgrp_dfl_root);
 	spin_unlock_irq(&css_set_lock);
 
-	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
-					    of->file->f_path.dentry->d_sb);
+	ret = cgroup_attach_permissions(src_cgrp, dst_cgrp,
+					of->file->f_path.dentry->d_sb, false);
 	if (ret)
 		goto out_finish;
 
@@ -4757,16 +4780,11 @@ static ssize_t cgroup_threads_write(struct kernfs_open_file *of,
 	spin_unlock_irq(&css_set_lock);
 
 	/* thread migrations follow the cgroup.procs delegation rule */
-	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
-					    of->file->f_path.dentry->d_sb);
+	ret = cgroup_attach_permissions(src_cgrp, dst_cgrp,
+					of->file->f_path.dentry->d_sb, true);
 	if (ret)
 		goto out_finish;
 
-	/* and must be contained in the same domain */
-	ret = -EOPNOTSUPP;
-	if (src_cgrp->dom_cgrp != dst_cgrp->dom_cgrp)
-		goto out_finish;
-
 	ret = cgroup_attach_task(dst_cgrp, task, false);
 
 out_finish:
-- 
2.25.0

^ permalink raw reply related

* [PATCH v3 2/5] cgroup: add cgroup_get_from_file() helper
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner, Johannes Weiner, Li Zefan,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200117002143.15559-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>

Add a helper cgroup_get_from_file(). The helper will be used in
subsequent patches to retrieve a cgroup while holding a reference to the
struct file it was taken from.

Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Christian Brauner <christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
/* v1 */
patch not present

/* v2 */
patch not present

/* v3 */
patch introduced
- Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
  - split cgroup_get_from_file() changes into separate commmit
---
 kernel/cgroup/cgroup.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ad1f9fea5c14..f05efc2677c8 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5882,6 +5882,24 @@ void cgroup_fork(struct task_struct *child)
 	INIT_LIST_HEAD(&child->cg_list);
 }
 
+static struct cgroup *cgroup_get_from_file(struct file *f)
+{
+	struct cgroup_subsys_state *css;
+	struct cgroup *cgrp;
+
+	css = css_tryget_online_from_dir(f->f_path.dentry, NULL);
+	if (IS_ERR(css))
+		return ERR_CAST(css);
+
+	cgrp = css->cgroup;
+	if (!cgroup_on_dfl(cgrp)) {
+		cgroup_put(cgrp);
+		return ERR_PTR(-EBADF);
+	}
+
+	return cgrp;
+}
+
 /**
  * cgroup_can_fork - called on a new task before the process is exposed
  * @child: the task in question.
@@ -6170,7 +6188,6 @@ EXPORT_SYMBOL_GPL(cgroup_get_from_path);
  */
 struct cgroup *cgroup_get_from_fd(int fd)
 {
-	struct cgroup_subsys_state *css;
 	struct cgroup *cgrp;
 	struct file *f;
 
@@ -6178,17 +6195,8 @@ struct cgroup *cgroup_get_from_fd(int fd)
 	if (!f)
 		return ERR_PTR(-EBADF);
 
-	css = css_tryget_online_from_dir(f->f_path.dentry, NULL);
+	cgrp = cgroup_get_from_file(f);
 	fput(f);
-	if (IS_ERR(css))
-		return ERR_CAST(css);
-
-	cgrp = css->cgroup;
-	if (!cgroup_on_dfl(cgrp)) {
-		cgroup_put(cgrp);
-		return ERR_PTR(-EBADF);
-	}
-
 	return cgrp;
 }
 EXPORT_SYMBOL_GPL(cgroup_get_from_fd);
-- 
2.25.0

^ permalink raw reply related

* [PATCH v3 3/5] cgroup: refactor fork helpers
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api, linux-kernel, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner, Johannes Weiner, Li Zefan,
	cgroups
In-Reply-To: <20200117002143.15559-1-christian.brauner@ubuntu.com>

This refactors the fork helpers so they can be easily modified in the
next patches. The patch just passes in the parent task_struct and moves
the cgroup threadgroup rwsem grab and release into the helpers. The
don't need to be directly exposed in fork.c.

Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: cgroups@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v1 */
patch not present

/* v2 */
patch not present

/* v3 */
patch introduced
- Tejun Heo <tj@kernel.org>:
  - split into separate commmit

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 include/linux/cgroup.h | 18 +++++++++-----
 kernel/cgroup/cgroup.c | 56 ++++++++++++++++++++++++++----------------
 kernel/fork.c          | 12 +++------
 3 files changed, 51 insertions(+), 35 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index d7ddebd0cdec..5349465bfac1 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -121,9 +121,12 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk);
 
 void cgroup_fork(struct task_struct *p);
-extern int cgroup_can_fork(struct task_struct *p);
-extern void cgroup_cancel_fork(struct task_struct *p);
-extern void cgroup_post_fork(struct task_struct *p);
+extern int cgroup_can_fork(struct task_struct *parent,
+			   struct task_struct *child);
+extern void cgroup_cancel_fork(struct task_struct *parent,
+			       struct task_struct *child);
+extern void cgroup_post_fork(struct task_struct *parent,
+			     struct task_struct *child);
 void cgroup_exit(struct task_struct *p);
 void cgroup_release(struct task_struct *p);
 void cgroup_free(struct task_struct *p);
@@ -707,9 +710,12 @@ static inline int cgroupstats_build(struct cgroupstats *stats,
 				    struct dentry *dentry) { return -EINVAL; }
 
 static inline void cgroup_fork(struct task_struct *p) {}
-static inline int cgroup_can_fork(struct task_struct *p) { return 0; }
-static inline void cgroup_cancel_fork(struct task_struct *p) {}
-static inline void cgroup_post_fork(struct task_struct *p) {}
+static inline int cgroup_can_fork(struct task_struct *parent,
+				  struct task_struct *child) { return 0; }
+static inline void cgroup_cancel_fork(struct task_struct *parent,
+			       struct task_struct *child) {};
+static inline void cgroup_post_fork(struct task_struct *parent,
+				    struct task_struct *child) {};
 static inline void cgroup_exit(struct task_struct *p) {}
 static inline void cgroup_release(struct task_struct *p) {}
 static inline void cgroup_free(struct task_struct *p) {}
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index f05efc2677c8..49d8cf087e10 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5902,17 +5902,22 @@ static struct cgroup *cgroup_get_from_file(struct file *f)
 
 /**
  * cgroup_can_fork - called on a new task before the process is exposed
- * @child: the task in question.
+ * @parent: the parent process of @child
+ * @child: the child process of @parent
+ * @kargs: the arguments passed to create the child process
  *
- * This calls the subsystem can_fork() callbacks. If the can_fork() callback
- * returns an error, the fork aborts with that error code. This allows for
- * a cgroup subsystem to conditionally allow or deny new forks.
+ * This calls the subsystem can_fork() callbacks. If the cgroup_can_fork()
+ * callback returns an error, the fork aborts with that error code. This
+ * allows for a cgroup subsystem to conditionally allow or deny new forks.
  */
-int cgroup_can_fork(struct task_struct *child)
+int cgroup_can_fork(struct task_struct *parent, struct task_struct *child)
+	__acquires(&cgroup_threadgroup_rwsem) __releases(&cgroup_threadgroup_rwsem)
 {
 	struct cgroup_subsys *ss;
 	int i, j, ret;
 
+	cgroup_threadgroup_change_begin(parent);
+
 	do_each_subsys_mask(ss, i, have_canfork_callback) {
 		ret = ss->can_fork(child);
 		if (ret)
@@ -5929,17 +5934,22 @@ int cgroup_can_fork(struct task_struct *child)
 			ss->cancel_fork(child);
 	}
 
+	cgroup_threadgroup_change_end(parent);
+
 	return ret;
 }
 
 /**
- * cgroup_cancel_fork - called if a fork failed after cgroup_can_fork()
- * @child: the task in question
- *
- * This calls the cancel_fork() callbacks if a fork failed *after*
- * cgroup_can_fork() succeded.
- */
-void cgroup_cancel_fork(struct task_struct *child)
+  * cgroup_cancel_fork - called if a fork failed after cgroup_can_fork()
+  * @parent: the parent process of @child
+  * @child: the child process of @parent
+  * @kargs: the arguments passed to create the child process
+  *
+  * This calls the cancel_fork() callbacks if a fork failed *after*
+  * cgroup_can_fork() succeded.
+  */
+void cgroup_cancel_fork(struct task_struct *parent, struct task_struct *child)
+	__releases(&cgroup_threadgroup_rwsem)
 {
 	struct cgroup_subsys *ss;
 	int i;
@@ -5947,19 +5957,21 @@ void cgroup_cancel_fork(struct task_struct *child)
 	for_each_subsys(ss, i)
 		if (ss->cancel_fork)
 			ss->cancel_fork(child);
+
+	cgroup_threadgroup_change_end(parent);
 }
 
 /**
- * cgroup_post_fork - called on a new task after adding it to the task list
- * @child: the task in question
- *
- * Adds the task to the list running through its css_set if necessary and
- * call the subsystem fork() callbacks.  Has to be after the task is
- * visible on the task list in case we race with the first call to
- * cgroup_task_iter_start() - to guarantee that the new task ends up on its
- * list.
+ * cgroup_post_fork - finalize cgroup setup for the child process
+ * @parent: the parent process of @child
+ * @child: the child process of @parent
+ * @kargs: the arguments passed to create the child process
+ *
+ * Attach the child process to its css_set calling the subsystem fork()
+ * callbacks.
  */
-void cgroup_post_fork(struct task_struct *child)
+void cgroup_post_fork(struct task_struct *parent, struct task_struct *child)
+	__releases(&cgroup_threadgroup_rwsem)
 {
 	struct cgroup_subsys *ss;
 	struct css_set *cset;
@@ -6002,6 +6014,8 @@ void cgroup_post_fork(struct task_struct *child)
 	do_each_subsys_mask(ss, i, have_fork_callback) {
 		ss->fork(child);
 	} while_each_subsys_mask();
+
+	cgroup_threadgroup_change_end(parent);
 }
 
 /**
diff --git a/kernel/fork.c b/kernel/fork.c
index 080809560072..c76758dbd594 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2165,16 +2165,15 @@ static __latent_entropy struct task_struct *copy_process(
 	INIT_LIST_HEAD(&p->thread_group);
 	p->task_works = NULL;
 
-	cgroup_threadgroup_change_begin(current);
 	/*
 	 * Ensure that the cgroup subsystem policies allow the new process to be
 	 * forked. It should be noted the the new process's css_set can be changed
 	 * between here and cgroup_post_fork() if an organisation operation is in
 	 * progress.
 	 */
-	retval = cgroup_can_fork(p);
+	retval = cgroup_can_fork(current, p);
 	if (retval)
-		goto bad_fork_cgroup_threadgroup_change_end;
+		goto bad_fork_put_pidfd;
 
 	/*
 	 * From this point on we must avoid any synchronous user-space
@@ -2279,8 +2278,7 @@ static __latent_entropy struct task_struct *copy_process(
 	write_unlock_irq(&tasklist_lock);
 
 	proc_fork_connector(p);
-	cgroup_post_fork(p);
-	cgroup_threadgroup_change_end(current);
+	cgroup_post_fork(current, p);
 	perf_event_fork(p);
 
 	trace_task_newtask(p, clone_flags);
@@ -2291,9 +2289,7 @@ static __latent_entropy struct task_struct *copy_process(
 bad_fork_cancel_cgroup:
 	spin_unlock(&current->sighand->siglock);
 	write_unlock_irq(&tasklist_lock);
-	cgroup_cancel_fork(p);
-bad_fork_cgroup_threadgroup_change_end:
-	cgroup_threadgroup_change_end(current);
+	cgroup_cancel_fork(current, p);
 bad_fork_put_pidfd:
 	if (clone_flags & CLONE_PIDFD) {
 		fput(pidfile);
-- 
2.25.0

^ permalink raw reply related

* [PATCH v3 4/5] clone3: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api, linux-kernel, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner, Ingo Molnar, Johannes Weiner,
	Li Zefan, Peter Zijlstra, cgroups
In-Reply-To: <20200117002143.15559-1-christian.brauner@ubuntu.com>

This adds support for creating a process in a different cgroup than its
parent. Callers can limit and account processes and threads right from
the moment they are spawned:
- A service manager can directly spawn new services into dedicated
  cgroups.
- A process can be directly created in a frozen cgroup and will be
  frozen as well.
- The initial accounting jitter experienced by process supervisors and
  daemons is eliminated with this.
- Threaded applications or even thread implementations can choose to
  create a specific cgroup layout where each thread is spawned
  directly into a dedicated cgroup.

This feature is limited to the unified hierarchy. Callers need to pass
an directory file descriptor for the target cgroup. The caller can
choose to pass an O_PATH file descriptor. All usual migration
restrictions apply, i.e. there can be no processes in inner nodes. In
general, creating a process directly in a target cgroup adheres to all
migration restrictions.

Cc: Tejun Heo <tj@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v1 */
Link: https://lore.kernel.org/r/20191218173516.7875-3-christian.brauner@ubuntu.com

/* v2 */
Link: https://lore.kernel.org/r/20191223061504.28716-3-christian.brauner@ubuntu.com
- Oleg Nesterov <oleg@redhat.com>:
  - prevent deadlock from wrong locking order
- Christian Brauner <christian.brauner@ubuntu.com>:
  - Rework locking. In the previous patch version we would have already
    acquired the cgroup_threadgroup_rwsem before we grabbed cgroup mutex
    we need to hold when CLONE_INTO_CGROUP is specified. This meant we
    could deadlock with other codepaths that all require it to be done
    the other way around. Fix this by first grabbing cgroup mutex when
    CLONE_INTO_CGROUP is specified and then grabbing
    cgroup_threadgroup_rwsem unconditionally after. This way we don't
    require the cgroup mutex be held in codepaths that don't need it.
  - Switch from mutex_lock() to mutex_lock_killable().

/* v3 */
- Tejun Heo <tj@kernel.org>:
  - s/mutex_lock_killable()/mutex_lock()/ because it should only ever
    be held for a short time:
    diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
    index a9fedcfeae4b..d68d3fb6af1d 100644
    --- a/kernel/cgroup/cgroup.c
    +++ b/kernel/cgroup/cgroup.c
    @@ -5927,11 +5927,8 @@ static int cgroup_css_set_fork(struct task_struct *parent,
            struct super_block *sb;
            struct file *f;

    -       if (kargs->flags & CLONE_INTO_CGROUP) {
    -               ret = mutex_lock_killable(&cgroup_mutex);
    -               if (ret)
    -                       return ret;
    -       }
    +       if (kargs->flags & CLONE_INTO_CGROUP)
    +               mutex_lock(&cgroup_mutex);

            cgroup_threadgroup_change_begin(parent);
  - s/task_cgroup_from_root/cset->dfl_cgrp/:
    diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
    index d68d3fb6af1d..3ceef006d144 100644
    --- a/kernel/cgroup/cgroup.c
    +++ b/kernel/cgroup/cgroup.c
    @@ -5922,7 +5922,7 @@ static int cgroup_css_set_fork(struct task_struct *parent,
            __acquires(&cgroup_mutex) __acquires(&cgroup_threadgroup_rwsem)
     {
            int ret;
    -       struct cgroup *dst_cgrp = NULL, *src_cgrp;
    +       struct cgroup *dst_cgrp = NULL;
            struct css_set *cset;
            struct super_block *sb;
            struct file *f;
    @@ -5956,11 +5956,7 @@ static int cgroup_css_set_fork(struct task_struct *parent,
                    goto err;
            }

    -       spin_lock_irq(&css_set_lock);
    -       src_cgrp = task_cgroup_from_root(parent, &cgrp_dfl_cgrp);
    -       spin_unlock_irq(&css_set_lock);
    -
    -       ret = cgroup_attach_permissions(src_cgrp, dst_cgrp, sb,
    +       ret = cgroup_attach_permissions(cset->dfl_cgrp, dst_cgrp, sb,
                                            !!(kargs->flags & CLONE_THREAD));
            if (ret)
                    goto err;
  - pass struct css_set instead of struct kernel_clone_args into cgroup
    fork subsystem callbacks:
    diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
    index cd848c6bac4a..058bb16d073f 100644
    --- a/include/linux/cgroup-defs.h
    +++ b/include/linux/cgroup-defs.h
    @@ -630,9 +630,8 @@ struct cgroup_subsys {
     	void (*attach)(struct cgroup_taskset *tset);
     	void (*post_attach)(void);
     	int (*can_fork)(struct task_struct *parent, struct task_struct *child,
    -			struct kernel_clone_args *kargs);
    -	void (*cancel_fork)(struct task_struct *child,
    -			    struct kernel_clone_args *kargs);
    +			struct css_set *cset);
    +	void (*cancel_fork)(struct task_struct *child, struct css_set *cset);
     	void (*fork)(struct task_struct *task);
     	void (*exit)(struct task_struct *task);
     	void (*release)(struct task_struct *task);
    diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
    index 3ceef006d144..2ac1c37a3fcb 100644
    --- a/kernel/cgroup/cgroup.c
    +++ b/kernel/cgroup/cgroup.c
    @@ -6044,7 +6044,7 @@ int cgroup_can_fork(struct task_struct *parent, struct task_struct *child,
     		return ret;

     	do_each_subsys_mask(ss, i, have_canfork_callback) {
    -		ret = ss->can_fork(parent, child, kargs);
    +		ret = ss->can_fork(parent, child, kargs->cset);
     		if (ret)
     			goto out_revert;
     	} while_each_subsys_mask();
    @@ -6056,7 +6056,7 @@ int cgroup_can_fork(struct task_struct *parent, struct task_struct *child,
     		if (j >= i)
     			break;
     		if (ss->cancel_fork)
    -			ss->cancel_fork(child, kargs);
    +			ss->cancel_fork(child, kargs->cset);
     	}

     	cgroup_css_set_put_fork(parent, kargs);
    @@ -6082,7 +6082,7 @@ void cgroup_cancel_fork(struct task_struct *parent, struct task_struct *child,

     	for_each_subsys(ss, i)
     		if (ss->cancel_fork)
    -			ss->cancel_fork(child, kargs);
    +			ss->cancel_fork(child, kargs->cset);

     	cgroup_css_set_put_fork(parent, kargs);
     }
    diff --git a/kernel/cgroup/pids.c b/kernel/cgroup/pids.c
    index e5955bc1fb00..4e7c8819c8df 100644
    --- a/kernel/cgroup/pids.c
    +++ b/kernel/cgroup/pids.c
    @@ -216,20 +216,16 @@ static void pids_cancel_attach(struct cgroup_taskset *tset)
      * on cgroup_threadgroup_change_begin() held by the copy_process().
      */
     static int pids_can_fork(struct task_struct *parent, struct task_struct *child,
    -			 struct kernel_clone_args *args)
    +			 struct css_set *cset)
     {
    -	struct css_set *new_cset = NULL;
     	struct cgroup_subsys_state *css;
     	struct pids_cgroup *pids;
     	int err;

    -	if (args)
    -		new_cset = args->cset;
    -
    -	if (!new_cset)
    -		css = task_css_check(current, pids_cgrp_id, true);
    +	if (cset)
    +		css = cset->subsys[pids_cgrp_id];
     	else
    -		css = new_cset->subsys[pids_cgrp_id];
    +		css = task_css_check(current, pids_cgrp_id, true);
     	pids = css_pids(css);
     	err = pids_try_charge(pids, 1);
     	if (err) {
    @@ -244,20 +240,15 @@ static int pids_can_fork(struct task_struct *parent, struct task_struct *child,
     	return err;
     }

    -static void pids_cancel_fork(struct task_struct *task,
    -			     struct kernel_clone_args *args)
    +static void pids_cancel_fork(struct task_struct *task, struct css_set *cset)
     {
    -	struct css_set *new_cset = NULL;
     	struct cgroup_subsys_state *css;
     	struct pids_cgroup *pids;

    -	if (args)
    -		new_cset = args->cset;
    -
    -	if (!new_cset)
    -		css = task_css_check(current, pids_cgrp_id, true);
    +	if (cset)
    +		css = cset->subsys[pids_cgrp_id];
     	else
    -		css = new_cset->subsys[pids_cgrp_id];
    +		css = task_css_check(current, pids_cgrp_id, true);
     	pids = css_pids(css);
     	pids_uncharge(pids, 1);
     }
- Michal Koutný <mkoutny@suse.com>:
  - update comment for cgroup_fork()
  - if CLONE_NEWCGROUP and CLONE_INTO_CGROUP is requested, set the
    root_cset of the new cgroup namespace to the child's cset
---
 include/linux/cgroup-defs.h |   6 +-
 include/linux/cgroup.h      |  20 ++--
 include/linux/sched/task.h  |   4 +
 include/uapi/linux/sched.h  |   5 +
 kernel/cgroup/cgroup.c      | 185 +++++++++++++++++++++++++++++++-----
 kernel/cgroup/pids.c        |  16 +++-
 kernel/fork.c               |  13 ++-
 7 files changed, 208 insertions(+), 41 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 63097cb243cb..058bb16d073f 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -33,6 +33,7 @@ struct kernfs_ops;
 struct kernfs_open_file;
 struct seq_file;
 struct poll_table_struct;
+struct kernel_clone_args;
 
 #define MAX_CGROUP_TYPE_NAMELEN 32
 #define MAX_CGROUP_ROOT_NAMELEN 64
@@ -628,8 +629,9 @@ struct cgroup_subsys {
 	void (*cancel_attach)(struct cgroup_taskset *tset);
 	void (*attach)(struct cgroup_taskset *tset);
 	void (*post_attach)(void);
-	int (*can_fork)(struct task_struct *task);
-	void (*cancel_fork)(struct task_struct *task);
+	int (*can_fork)(struct task_struct *parent, struct task_struct *child,
+			struct css_set *cset);
+	void (*cancel_fork)(struct task_struct *child, struct css_set *cset);
 	void (*fork)(struct task_struct *task);
 	void (*exit)(struct task_struct *task);
 	void (*release)(struct task_struct *task);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 5349465bfac1..dfd5e095f4ee 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -27,6 +27,8 @@
 
 #include <linux/cgroup-defs.h>
 
+struct kernel_clone_args;
+
 #ifdef CONFIG_CGROUPS
 
 /*
@@ -122,11 +124,14 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 
 void cgroup_fork(struct task_struct *p);
 extern int cgroup_can_fork(struct task_struct *parent,
-			   struct task_struct *child);
+			   struct task_struct *child,
+			   struct kernel_clone_args *kargs);
 extern void cgroup_cancel_fork(struct task_struct *parent,
-			       struct task_struct *child);
+			       struct task_struct *child,
+			       struct kernel_clone_args *kargs);
 extern void cgroup_post_fork(struct task_struct *parent,
-			     struct task_struct *child);
+			     struct task_struct *child,
+			     struct kernel_clone_args *kargs);
 void cgroup_exit(struct task_struct *p);
 void cgroup_release(struct task_struct *p);
 void cgroup_free(struct task_struct *p);
@@ -711,11 +716,14 @@ static inline int cgroupstats_build(struct cgroupstats *stats,
 
 static inline void cgroup_fork(struct task_struct *p) {}
 static inline int cgroup_can_fork(struct task_struct *parent,
-				  struct task_struct *child) { return 0; }
+				  struct task_struct *child,
+				  struct kernel_clone_args *kargs) { return 0; }
 static inline void cgroup_cancel_fork(struct task_struct *parent,
-			       struct task_struct *child) {};
+				      struct task_struct *child,
+				      struct kernel_clone_args *kargs) {};
 static inline void cgroup_post_fork(struct task_struct *parent,
-				    struct task_struct *child) {};
+				    struct task_struct *child,
+				    struct kernel_clone_args *kargs) {};
 static inline void cgroup_exit(struct task_struct *p) {}
 static inline void cgroup_release(struct task_struct *p) {}
 static inline void cgroup_free(struct task_struct *p) {}
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index f1879884238e..38359071236a 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -13,6 +13,7 @@
 struct task_struct;
 struct rusage;
 union thread_union;
+struct css_set;
 
 /* All the bits taken by the old clone syscall. */
 #define CLONE_LEGACY_FLAGS 0xffffffffULL
@@ -29,6 +30,9 @@ struct kernel_clone_args {
 	pid_t *set_tid;
 	/* Number of elements in *set_tid */
 	size_t set_tid_size;
+	int cgroup;
+	struct cgroup *cgrp;
+	struct css_set *cset;
 };
 
 /*
diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 4a0217832464..08620c220f30 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -35,6 +35,7 @@
 
 /* Flags for the clone3() syscall. */
 #define CLONE_CLEAR_SIGHAND 0x100000000ULL /* Clear any signal handler and reset to SIG_DFL. */
+#define CLONE_INTO_CGROUP 0x200000000ULL /* Clone into a specific cgroup given the right permissions. */
 
 #ifndef __ASSEMBLY__
 /**
@@ -75,6 +76,8 @@
  * @set_tid_size: This defines the size of the array referenced
  *                in @set_tid. This cannot be larger than the
  *                kernel's limit of nested PID namespaces.
+ * @cgroup:       If CLONE_INTO_CGROUP is specified set this to
+ *                a file descriptor for the cgroup.
  *
  * The structure is versioned by size and thus extensible.
  * New struct members must go at the end of the struct and
@@ -91,11 +94,13 @@ struct clone_args {
 	__aligned_u64 tls;
 	__aligned_u64 set_tid;
 	__aligned_u64 set_tid_size;
+	__aligned_u64 cgroup;
 };
 #endif
 
 #define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */
 #define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */
+#define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */
 
 /*
  * Scheduling policies
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 49d8cf087e10..adf82c306dce 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5873,8 +5873,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
  * @child: pointer to task_struct of forking parent process.
  *
  * A task is associated with the init_css_set until cgroup_post_fork()
- * attaches it to the parent's css_set.  Empty cg_list indicates that
- * @child isn't holding reference to its css_set.
+ * attaches it to the target css_set.
  */
 void cgroup_fork(struct task_struct *child)
 {
@@ -5900,26 +5899,151 @@ static struct cgroup *cgroup_get_from_file(struct file *f)
 	return cgrp;
 }
 
+/**
+ * cgroup_css_set_fork - find or create a css_set for a child process
+ * @parent: the parent of the child process
+ * @kargs: the arguments passed to create the child process
+ *
+ * This functions finds or creates a new css_set which the child
+ * process will be attached to in cgroup_post_fork(). By default,
+ * the child process will be given the same css_set as its parent.
+ *
+ * If CLONE_INTO_CGROUP is specified this function will try to find an
+ * existing css_set which includes the request cgroup and if not create
+ * a new css_set that the child will be attached to. If this function
+ * succeeds it will hold cgroup_threadgroup_rwsem on return. If
+ * CLONE_INTO_CGROUP is requested this function will grab cgroup mutex
+ * before grabbing cgroup_threadgroup_rwsem and will hold a reference
+ * to the target cgroup.
+ */
+static int cgroup_css_set_fork(struct task_struct *parent,
+			       struct kernel_clone_args *kargs)
+	__acquires(&cgroup_mutex) __acquires(&cgroup_threadgroup_rwsem)
+{
+	int ret;
+	struct cgroup *dst_cgrp = NULL;
+	struct css_set *cset;
+	struct super_block *sb;
+	struct file *f;
+
+	if (kargs->flags & CLONE_INTO_CGROUP)
+		mutex_lock(&cgroup_mutex);
+
+	cgroup_threadgroup_change_begin(parent);
+
+	spin_lock_irq(&css_set_lock);
+	cset = task_css_set(parent);
+	get_css_set(cset);
+	spin_unlock_irq(&css_set_lock);
+
+	if (!(kargs->flags & CLONE_INTO_CGROUP)) {
+		kargs->cset = cset;
+		return 0;
+	}
+
+	f = fget_raw(kargs->cgroup);
+	if (!f) {
+		ret = -EBADF;
+		goto err;
+	}
+	sb = f->f_path.dentry->d_sb;
+
+	dst_cgrp = cgroup_get_from_file(f);
+	if (IS_ERR(dst_cgrp)) {
+		ret = PTR_ERR(dst_cgrp);
+		dst_cgrp = NULL;
+		goto err;
+	}
+
+	ret = cgroup_attach_permissions(cset->dfl_cgrp, dst_cgrp, sb,
+					!!(kargs->flags & CLONE_THREAD));
+	if (ret)
+		goto err;
+
+	kargs->cset = find_css_set(cset, dst_cgrp);
+	if (!kargs->cset) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	if (cgroup_is_dead(dst_cgrp)) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	put_css_set(cset);
+	fput(f);
+	kargs->cgrp = dst_cgrp;
+	return ret;
+
+err:
+	cgroup_threadgroup_change_end(parent);
+	mutex_unlock(&cgroup_mutex);
+	if (f)
+		fput(f);
+	if (dst_cgrp)
+		cgroup_put(dst_cgrp);
+	put_css_set(cset);
+	return ret;
+}
+
+/**
+ * cgroup_css_set_put_fork - drop references we took during fork
+ * @parent: the parent of the child process
+ * @kargs: the arguments passed to create the child process
+ *
+ * Drop references to the prepared css_set and target cgroup if
+ * CLONE_INTO_CGROUP was requested. This function can only be
+ * called before fork()'s point of no return.
+ */
+static void cgroup_css_set_put_fork(struct task_struct *parent,
+				    struct kernel_clone_args *kargs)
+	__releases(&cgroup_threadgroup_rwsem) __releases(&cgroup_mutex)
+{
+	cgroup_threadgroup_change_end(parent);
+
+	if (kargs->flags & CLONE_INTO_CGROUP) {
+		struct cgroup *cgrp = kargs->cgrp;
+		struct css_set *cset = kargs->cset;
+
+		mutex_unlock(&cgroup_mutex);
+
+		if (cset) {
+			put_css_set(cset);
+			kargs->cset = NULL;
+		}
+
+		if (cgrp) {
+			cgroup_put(cgrp);
+			kargs->cgrp = NULL;
+		}
+	}
+}
+
 /**
  * cgroup_can_fork - called on a new task before the process is exposed
  * @parent: the parent process of @child
  * @child: the child process of @parent
  * @kargs: the arguments passed to create the child process
  *
+ * This prepares a new css_set for the child process which the child will
+ * be attached to in cgroup_post_fork().
  * This calls the subsystem can_fork() callbacks. If the cgroup_can_fork()
  * callback returns an error, the fork aborts with that error code. This
  * allows for a cgroup subsystem to conditionally allow or deny new forks.
  */
-int cgroup_can_fork(struct task_struct *parent, struct task_struct *child)
-	__acquires(&cgroup_threadgroup_rwsem) __releases(&cgroup_threadgroup_rwsem)
+int cgroup_can_fork(struct task_struct *parent, struct task_struct *child,
+			struct kernel_clone_args *kargs)
 {
 	struct cgroup_subsys *ss;
 	int i, j, ret;
 
-	cgroup_threadgroup_change_begin(parent);
+	ret = cgroup_css_set_fork(parent, kargs);
+	if (ret)
+		return ret;
 
 	do_each_subsys_mask(ss, i, have_canfork_callback) {
-		ret = ss->can_fork(child);
+		ret = ss->can_fork(parent, child, kargs->cset);
 		if (ret)
 			goto out_revert;
 	} while_each_subsys_mask();
@@ -5931,34 +6055,35 @@ int cgroup_can_fork(struct task_struct *parent, struct task_struct *child)
 		if (j >= i)
 			break;
 		if (ss->cancel_fork)
-			ss->cancel_fork(child);
+			ss->cancel_fork(child, kargs->cset);
 	}
 
-	cgroup_threadgroup_change_end(parent);
+	cgroup_css_set_put_fork(parent, kargs);
 
 	return ret;
 }
 
 /**
-  * cgroup_cancel_fork - called if a fork failed after cgroup_can_fork()
-  * @parent: the parent process of @child
-  * @child: the child process of @parent
-  * @kargs: the arguments passed to create the child process
-  *
-  * This calls the cancel_fork() callbacks if a fork failed *after*
-  * cgroup_can_fork() succeded.
-  */
-void cgroup_cancel_fork(struct task_struct *parent, struct task_struct *child)
-	__releases(&cgroup_threadgroup_rwsem)
+ * cgroup_cancel_fork - called if a fork failed after cgroup_can_fork()
+ * @parent: the parent process of @child
+ * @child: the child process of @parent
+ * @kargs: the arguments passed to create the child process
+ *
+ * This calls the cancel_fork() callbacks if a fork failed *after*
+ * cgroup_can_fork() succeded and cleans up references we took to
+ * prepare a new css_set for the child process in cgroup_can_fork().
+ */
+void cgroup_cancel_fork(struct task_struct *parent, struct task_struct *child,
+			struct kernel_clone_args *kargs)
 {
 	struct cgroup_subsys *ss;
 	int i;
 
 	for_each_subsys(ss, i)
 		if (ss->cancel_fork)
-			ss->cancel_fork(child);
+			ss->cancel_fork(child, kargs->cset);
 
-	cgroup_threadgroup_change_end(parent);
+	cgroup_css_set_put_fork(parent, kargs);
 }
 
 /**
@@ -5970,18 +6095,17 @@ void cgroup_cancel_fork(struct task_struct *parent, struct task_struct *child)
  * Attach the child process to its css_set calling the subsystem fork()
  * callbacks.
  */
-void cgroup_post_fork(struct task_struct *parent, struct task_struct *child)
-	__releases(&cgroup_threadgroup_rwsem)
+void cgroup_post_fork(struct task_struct *parent, struct task_struct *child,
+		      struct kernel_clone_args *kargs)
+	__releases(&cgroup_threadgroup_rwsem) __releases(&cgroup_mutex)
 {
 	struct cgroup_subsys *ss;
-	struct css_set *cset;
+	struct css_set *cset = kargs->cset;
 	int i;
 
 	spin_lock_irq(&css_set_lock);
 
 	WARN_ON_ONCE(!list_empty(&child->cg_list));
-	cset = task_css_set(current); /* current is @child's parent */
-	get_css_set(cset);
 	cset->nr_tasks++;
 	css_set_move_task(child, NULL, cset, false);
 
@@ -6016,6 +6140,17 @@ void cgroup_post_fork(struct task_struct *parent, struct task_struct *child)
 	} while_each_subsys_mask();
 
 	cgroup_threadgroup_change_end(parent);
+
+	if (kargs->flags & CLONE_INTO_CGROUP) {
+		mutex_unlock(&cgroup_mutex);
+
+		cgroup_put(kargs->cgrp);
+		kargs->cgrp = NULL;
+	}
+
+	/* Make the new cset the root_cset of the new cgroup namespace. */
+	if (kargs->flags & CLONE_NEWCGROUP)
+		child->nsproxy->cgroup_ns->root_cset = cset;
 }
 
 /**
diff --git a/kernel/cgroup/pids.c b/kernel/cgroup/pids.c
index 138059eb730d..4e7c8819c8df 100644
--- a/kernel/cgroup/pids.c
+++ b/kernel/cgroup/pids.c
@@ -33,6 +33,7 @@
 #include <linux/atomic.h>
 #include <linux/cgroup.h>
 #include <linux/slab.h>
+#include <linux/sched/task.h>
 
 #define PIDS_MAX (PID_MAX_LIMIT + 1ULL)
 #define PIDS_MAX_STR "max"
@@ -214,13 +215,17 @@ static void pids_cancel_attach(struct cgroup_taskset *tset)
  * task_css_check(true) in pids_can_fork() and pids_cancel_fork() relies
  * on cgroup_threadgroup_change_begin() held by the copy_process().
  */
-static int pids_can_fork(struct task_struct *task)
+static int pids_can_fork(struct task_struct *parent, struct task_struct *child,
+			 struct css_set *cset)
 {
 	struct cgroup_subsys_state *css;
 	struct pids_cgroup *pids;
 	int err;
 
-	css = task_css_check(current, pids_cgrp_id, true);
+	if (cset)
+		css = cset->subsys[pids_cgrp_id];
+	else
+		css = task_css_check(current, pids_cgrp_id, true);
 	pids = css_pids(css);
 	err = pids_try_charge(pids, 1);
 	if (err) {
@@ -235,12 +240,15 @@ static int pids_can_fork(struct task_struct *task)
 	return err;
 }
 
-static void pids_cancel_fork(struct task_struct *task)
+static void pids_cancel_fork(struct task_struct *task, struct css_set *cset)
 {
 	struct cgroup_subsys_state *css;
 	struct pids_cgroup *pids;
 
-	css = task_css_check(current, pids_cgrp_id, true);
+	if (cset)
+		css = cset->subsys[pids_cgrp_id];
+	else
+		css = task_css_check(current, pids_cgrp_id, true);
 	pids = css_pids(css);
 	pids_uncharge(pids, 1);
 }
diff --git a/kernel/fork.c b/kernel/fork.c
index c76758dbd594..15d6576ad8c0 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2171,7 +2171,7 @@ static __latent_entropy struct task_struct *copy_process(
 	 * between here and cgroup_post_fork() if an organisation operation is in
 	 * progress.
 	 */
-	retval = cgroup_can_fork(current, p);
+	retval = cgroup_can_fork(current, p, args);
 	if (retval)
 		goto bad_fork_put_pidfd;
 
@@ -2278,7 +2278,7 @@ static __latent_entropy struct task_struct *copy_process(
 	write_unlock_irq(&tasklist_lock);
 
 	proc_fork_connector(p);
-	cgroup_post_fork(current, p);
+	cgroup_post_fork(current, p, args);
 	perf_event_fork(p);
 
 	trace_task_newtask(p, clone_flags);
@@ -2289,7 +2289,7 @@ static __latent_entropy struct task_struct *copy_process(
 bad_fork_cancel_cgroup:
 	spin_unlock(&current->sighand->siglock);
 	write_unlock_irq(&tasklist_lock);
-	cgroup_cancel_fork(current, p);
+	cgroup_cancel_fork(current, p, args);
 bad_fork_put_pidfd:
 	if (clone_flags & CLONE_PIDFD) {
 		fput(pidfile);
@@ -2618,6 +2618,9 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs,
 		     !valid_signal(args.exit_signal)))
 		return -EINVAL;
 
+	if ((args.flags & CLONE_INTO_CGROUP) && args.cgroup < 0)
+		return -EINVAL;
+
 	*kargs = (struct kernel_clone_args){
 		.flags		= args.flags,
 		.pidfd		= u64_to_user_ptr(args.pidfd),
@@ -2628,6 +2631,7 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs,
 		.stack_size	= args.stack_size,
 		.tls		= args.tls,
 		.set_tid_size	= args.set_tid_size,
+		.cgroup		= args.cgroup,
 	};
 
 	if (args.set_tid &&
@@ -2671,7 +2675,8 @@ static inline bool clone3_stack_valid(struct kernel_clone_args *kargs)
 static bool clone3_args_valid(struct kernel_clone_args *kargs)
 {
 	/* Verify that no unknown flags are passed along. */
-	if (kargs->flags & ~(CLONE_LEGACY_FLAGS | CLONE_CLEAR_SIGHAND))
+	if (kargs->flags &
+	    ~(CLONE_LEGACY_FLAGS | CLONE_CLEAR_SIGHAND | CLONE_INTO_CGROUP))
 		return false;
 
 	/*
-- 
2.25.0

^ permalink raw reply related

* [PATCH v3 5/5] selftests/cgroup: add tests for cloning into cgroups
From: Christian Brauner @ 2020-01-17  0:21 UTC (permalink / raw)
  To: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
  Cc: Oleg Nesterov, Christian Brauner, Roman Gushchin, Shuah Khan,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kselftest-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200117002143.15559-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>

Expand the cgroup test-suite to include tests for CLONE_INTO_CGROUP.
This adds the following tests:
- CLONE_INTO_CGROUP manages to clone a process directly into a correctly
  delegated cgroup
- CLONE_INTO_CGROUP fails to clone a process into a cgroup that has been
  removed after we've opened an fd to it
- CLONE_INTO_CGROUP fails to clone a process into an invalid domain
  cgroup
- CLONE_INTO_CGROUP adheres to the no internal process constraint
- CLONE_INTO_CGROUP works with the freezer feature

Cc: Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Shuah Khan <shuah-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kselftest-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Christian Brauner <christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
/* v1 */
Link: https://lore.kernel.org/r/20191218173516.7875-4-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org

/* v2 */
Link: https://lore.kernel.org/r/20191223061504.28716-4-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
unchanged

/* v3 */
unchanged
---
 tools/testing/selftests/cgroup/Makefile       |   6 +-
 tools/testing/selftests/cgroup/cgroup_util.c  | 126 ++++++++++++++++++
 tools/testing/selftests/cgroup/cgroup_util.h  |   4 +
 tools/testing/selftests/cgroup/test_core.c    |  64 +++++++++
 .../selftests/clone3/clone3_selftests.h       |  19 ++-
 5 files changed, 214 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index 66aafe1f5746..967f268fde74 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -11,6 +11,6 @@ TEST_GEN_PROGS += test_freezer
 
 include ../lib.mk
 
-$(OUTPUT)/test_memcontrol: cgroup_util.c
-$(OUTPUT)/test_core: cgroup_util.c
-$(OUTPUT)/test_freezer: cgroup_util.c
+$(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
+$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
+$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 8f7131dcf1ff..8a637ca7d73a 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -15,6 +15,7 @@
 #include <unistd.h>
 
 #include "cgroup_util.h"
+#include "../clone3/clone3_selftests.h"
 
 static ssize_t read_text(const char *path, char *buf, size_t max_len)
 {
@@ -331,12 +332,112 @@ int cg_run(const char *cgroup,
 	}
 }
 
+pid_t clone_into_cgroup(int cgroup_fd)
+{
+#ifdef CLONE_ARGS_SIZE_VER2
+	pid_t pid;
+
+	struct clone_args args = {
+		.flags = CLONE_INTO_CGROUP,
+		.exit_signal = SIGCHLD,
+		.cgroup = cgroup_fd,
+	};
+
+	pid = sys_clone3(&args, sizeof(struct clone_args));
+	/*
+	 * Verify that this is a genuine test failure:
+	 * ENOSYS -> clone3() not available
+	 * E2BIG  -> CLONE_INTO_CGROUP not available
+	 */
+	if (pid < 0 && (errno == ENOSYS || errno == E2BIG))
+		goto pretend_enosys;
+
+	return pid;
+
+pretend_enosys:
+#endif
+	errno = ENOSYS;
+	return -ENOSYS;
+}
+
+int clone_reap(pid_t pid, int options)
+{
+	int ret;
+	siginfo_t info = {
+		.si_signo = 0,
+	};
+
+again:
+	ret = waitid(P_PID, pid, &info, options | __WALL | __WNOTHREAD);
+	if (ret < 0) {
+		if (errno == EINTR)
+			goto again;
+		return -1;
+	}
+
+	if (options & WEXITED) {
+		if (WIFEXITED(info.si_status))
+			return WEXITSTATUS(info.si_status);
+	}
+
+	if (options & WSTOPPED) {
+		if (WIFSTOPPED(info.si_status))
+			return WSTOPSIG(info.si_status);
+	}
+
+	if (options & WCONTINUED) {
+		if (WIFCONTINUED(info.si_status))
+			return 0;
+	}
+
+	return -1;
+}
+
+int dirfd_open_opath(const char *dir)
+{
+	return open(dir, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW | O_PATH);
+}
+
+#define close_prot_errno(fd)                                                   \
+	if (fd >= 0) {                                                         \
+		int _e_ = errno;                                               \
+		close(fd);                                                     \
+		errno = _e_;                                                   \
+	}
+
+static int clone_into_cgroup_run_nowait(const char *cgroup,
+					int (*fn)(const char *cgroup, void *arg),
+					void *arg)
+{
+	int cgroup_fd;
+	pid_t pid;
+
+	cgroup_fd =  dirfd_open_opath(cgroup);
+	if (cgroup_fd < 0)
+		return -1;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	close_prot_errno(cgroup_fd);
+	if (pid == 0)
+		exit(fn(cgroup, arg));
+
+	return pid;
+}
+
 int cg_run_nowait(const char *cgroup,
 		  int (*fn)(const char *cgroup, void *arg),
 		  void *arg)
 {
 	int pid;
 
+	pid = clone_into_cgroup_run_nowait(cgroup, fn, arg);
+	if (pid > 0)
+		return pid;
+
+	/* Genuine test failure. */
+	if (pid < 0 && errno != ENOSYS)
+		return -1;
+
 	pid = fork();
 	if (pid == 0) {
 		char buf[64];
@@ -450,3 +551,28 @@ int proc_read_strstr(int pid, bool thread, const char *item, const char *needle)
 
 	return strstr(buf, needle) ? 0 : -1;
 }
+
+int clone_into_cgroup_run_wait(const char *cgroup)
+{
+	int cgroup_fd;
+	pid_t pid;
+
+	cgroup_fd =  dirfd_open_opath(cgroup);
+	if (cgroup_fd < 0)
+		return -1;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	close_prot_errno(cgroup_fd);
+	if (pid < 0)
+		return -1;
+
+	if (pid == 0)
+		exit(EXIT_SUCCESS);
+
+	/*
+	 * We don't care whether this fails. We only care whether the initial
+	 * clone succeeded.
+	 */
+	(void)clone_reap(pid, WEXITED);
+	return 0;
+}
diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
index 49c54fbdb229..5a1305dd1f0b 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/cgroup_util.h
@@ -50,3 +50,7 @@ extern int cg_wait_for_proc_count(const char *cgroup, int count);
 extern int cg_killall(const char *cgroup);
 extern ssize_t proc_read_text(int pid, bool thread, const char *item, char *buf, size_t size);
 extern int proc_read_strstr(int pid, bool thread, const char *item, const char *needle);
+extern pid_t clone_into_cgroup(int cgroup_fd);
+extern int clone_reap(pid_t pid, int options);
+extern int clone_into_cgroup_run_wait(const char *cgroup);
+extern int dirfd_open_opath(const char *dir);
diff --git a/tools/testing/selftests/cgroup/test_core.c b/tools/testing/selftests/cgroup/test_core.c
index c5ca669feb2b..96e016ccafe0 100644
--- a/tools/testing/selftests/cgroup/test_core.c
+++ b/tools/testing/selftests/cgroup/test_core.c
@@ -25,8 +25,11 @@
 static int test_cgcore_populated(const char *root)
 {
 	int ret = KSFT_FAIL;
+	int err;
 	char *cg_test_a = NULL, *cg_test_b = NULL;
 	char *cg_test_c = NULL, *cg_test_d = NULL;
+	int cgroup_fd = -EBADF;
+	pid_t pid;
 
 	cg_test_a = cg_name(root, "cg_test_a");
 	cg_test_b = cg_name(root, "cg_test_a/cg_test_b");
@@ -78,6 +81,52 @@ static int test_cgcore_populated(const char *root)
 	if (cg_read_strcmp(cg_test_d, "cgroup.events", "populated 0\n"))
 		goto cleanup;
 
+	/* Test that we can directly clone into a new cgroup. */
+	cgroup_fd = dirfd_open_opath(cg_test_d);
+	if (cgroup_fd < 0)
+		goto cleanup;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	if (pid < 0) {
+		if (errno == ENOSYS)
+			goto cleanup_pass;
+		goto cleanup;
+	}
+
+	if (pid == 0) {
+		if (raise(SIGSTOP))
+			exit(EXIT_FAILURE);
+		exit(EXIT_SUCCESS);
+	}
+
+	err = cg_read_strcmp(cg_test_d, "cgroup.events", "populated 1\n");
+
+	(void)clone_reap(pid, WSTOPPED);
+	(void)kill(pid, SIGCONT);
+	(void)clone_reap(pid, WEXITED);
+
+	if (err)
+		goto cleanup;
+
+	if (cg_read_strcmp(cg_test_d, "cgroup.events", "populated 0\n"))
+		goto cleanup;
+
+	/* Remove cgroup. */
+	if (cg_test_d) {
+		cg_destroy(cg_test_d);
+		free(cg_test_d);
+		cg_test_d = NULL;
+	}
+
+	pid = clone_into_cgroup(cgroup_fd);
+	if (pid < 0)
+		goto cleanup_pass;
+	if (pid == 0)
+		exit(EXIT_SUCCESS);
+	(void)clone_reap(pid, WEXITED);
+	goto cleanup;
+
+cleanup_pass:
 	ret = KSFT_PASS;
 
 cleanup:
@@ -93,6 +142,8 @@ static int test_cgcore_populated(const char *root)
 	free(cg_test_c);
 	free(cg_test_b);
 	free(cg_test_a);
+	if (cgroup_fd >= 0)
+		close(cgroup_fd);
 	return ret;
 }
 
@@ -136,6 +187,16 @@ static int test_cgcore_invalid_domain(const char *root)
 	if (errno != EOPNOTSUPP)
 		goto cleanup;
 
+	if (!clone_into_cgroup_run_wait(child))
+		goto cleanup;
+
+	if (errno == ENOSYS)
+		goto cleanup_pass;
+
+	if (errno != EOPNOTSUPP)
+		goto cleanup;
+
+cleanup_pass:
 	ret = KSFT_PASS;
 
 cleanup:
@@ -345,6 +406,9 @@ static int test_cgcore_internal_process_constraint(const char *root)
 	if (!cg_enter_current(parent))
 		goto cleanup;
 
+	if (!clone_into_cgroup_run_wait(parent))
+		goto cleanup;
+
 	ret = KSFT_PASS;
 
 cleanup:
diff --git a/tools/testing/selftests/clone3/clone3_selftests.h b/tools/testing/selftests/clone3/clone3_selftests.h
index a3f2c8ad8bcc..91c1a78ddb39 100644
--- a/tools/testing/selftests/clone3/clone3_selftests.h
+++ b/tools/testing/selftests/clone3/clone3_selftests.h
@@ -5,12 +5,24 @@
 
 #define _GNU_SOURCE
 #include <sched.h>
+#include <linux/sched.h>
+#include <linux/types.h>
 #include <stdint.h>
 #include <syscall.h>
-#include <linux/types.h>
+#include <sys/wait.h>
+
+#include "../kselftest.h"
 
 #define ptr_to_u64(ptr) ((__u64)((uintptr_t)(ptr)))
 
+#ifndef CLONE_INTO_CGROUP
+#define CLONE_INTO_CGROUP 0x200000000ULL /* Clone into a specific cgroup given the right permissions. */
+#endif
+
+#ifndef CLONE_ARGS_SIZE_VER0
+#define CLONE_ARGS_SIZE_VER0 64
+#endif
+
 #ifndef __NR_clone3
 #define __NR_clone3 -1
 struct clone_args {
@@ -22,10 +34,13 @@ struct clone_args {
 	__aligned_u64 stack;
 	__aligned_u64 stack_size;
 	__aligned_u64 tls;
+#define CLONE_ARGS_SIZE_VER1 80
 	__aligned_u64 set_tid;
 	__aligned_u64 set_tid_size;
+#define CLONE_ARGS_SIZE_VER2 88
+	__aligned_u64 cgroup;
 };
-#endif
+#endif /* __NR_clone3 */
 
 static pid_t sys_clone3(struct clone_args *args, size_t size)
 {
-- 
2.25.0

^ permalink raw reply related

* [RESEND Patch v6 0/2] add performance reporting support to FPGA DFL drivers
From: Wu Hao @ 2020-01-17  3:10 UTC (permalink / raw)
  To: will-DgEjT+Ai2ygdnm+yROfE0A, mdf-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-fpga-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, atull-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, Wu Hao

Hi all,

This patchset adds performance reporting support for FPGA DFL drivers. It
introduces one pmu to expose userspace interfaces via standard perf API.
User could use standard perf tool to access perf events exposed via pmu.

This patchset is generated based on latest fpga/for-next branch.

Main changes from v5:
 - use dev_ext_attribute instead of fme_perf_event_attr.
 - use is_visible function to decide which events to expose per
   hardware capability, and add event_init checking for all events.

Main changes from v4:
 - rebase and clean up.
 - update Kconfig for PERF_EVENTS dependency.

Main changes from v3:
 - add more descriptions in doc, including how to use perf tool for these
   hardware counters. (patch #1)
 - use standard perf API instead of sysfs entries. (patch #2)

Wu Hao (1):
  fpga: dfl: fme: add performance reporting support

Xu Yilun (1):
  Documentation: fpga: dfl: add description for performance reporting
    support

 Documentation/fpga/dfl.rst  |  83 ++++
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-fme-main.c |   4 +
 drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl-fme.h      |   2 +
 drivers/fpga/dfl.h          |   2 +
 6 files changed, 1035 insertions(+)
 create mode 100644 drivers/fpga/dfl-fme-perf.c

-- 
1.8.3.1

^ permalink raw reply

* [RESEND Patch v6 1/2] Documentation: fpga: dfl: add description for performance reporting support
From: Wu Hao @ 2020-01-17  3:10 UTC (permalink / raw)
  To: will-DgEjT+Ai2ygdnm+yROfE0A, mdf-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-fpga-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, atull-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, Xu Yilun, Wu Hao
In-Reply-To: <1579230628-22243-1-git-send-email-hao.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

From: Xu Yilun <yilun.xu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds description for performance reporting support for
Device Feature List (DFL) based FPGA.

Signed-off-by: Xu Yilun <yilun.xu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Wu Hao <hao.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
v5: rebase due to format change (txt->rst).
---
 Documentation/fpga/dfl.rst | 83 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 094fc8a..85b137b 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -118,6 +118,11 @@ More functions are exposed through sysfs
      management information (current temperature, thresholds, threshold status,
      etc.).
 
+ Performance reporting
+     performance counters are exposed through perf PMU APIs. Standard perf tool
+     can be used to monitor all available perf events. Please see performance
+     counter section below for more detailed information.
+
 
 FIU - PORT
 ==========
@@ -378,6 +383,84 @@ The device nodes used for ioctl() or mmap() can be referenced through::
 	/sys/class/fpga_region/<regionX>/<dfl-port.n>/dev
 
 
+Performance Counters
+====================
+Performance reporting is one private feature implemented in FME. It could
+supports several independent, system-wide, device counter sets in hardware to
+monitor and count for performance events, including "basic", "cache", "fabric",
+"vtd" and "vtd_sip" counters. Users could use standard perf tool to monitor
+FPGA cache hit/miss rate, transaction number, interface clock counter of AFU
+and other FPGA performance events.
+
+Different FPGA devices may have different counter sets, it depends on hardware
+implementation. e.g. some discrete FPGA cards don't have any cache. User could
+use "perf list" to check which perf events are supported by target hardware.
+
+In order to allow user to use standard perf API to access these performance
+counters, driver creates a perf PMU, and related sysfs interfaces in
+/sys/bus/event_source/devices/fme* to describe available perf events and
+configuration options.
+
+The "format" directory describes the format of the config field of struct
+perf_event_attr. There are 3 bitfields for config, "evtype" defines which type
+the perf event belongs to. "event" is the identity of the event within its
+category. "portid" is introduced to decide counters set to monitor on FPGA
+overall data or a specific port.
+
+The "events" directory describes the configuration templates for all available
+events which can be used with perf tool directly. For example, fab_mmio_read
+has the configuration "event=0x06,evtype=0x02,portid=0xff", which shows this
+event belongs to fabric type (0x02), the local event id is 0x06 and it is for
+overall monitoring (portid=0xff).
+
+Example usage of perf::
+
+  $# perf list |grep fme
+
+  fme0/fab_mmio_read/                              [Kernel PMU event]
+  <...>
+  fme0/fab_port_mmio_read,portid=?/                [Kernel PMU event]
+  <...>
+
+  $# perf stat -a -e fme0/fab_mmio_read/ <command>
+  or
+  $# perf stat -a -e fme0/event=0x06,evtype=0x02,portid=0xff/ <command>
+  or
+  $# perf stat -a -e fme0/config=0xff2006/ <command>
+
+Another example, fab_port_mmio_read monitors mmio read of a specific port. So
+its configuration template is "event=0x06,evtype=0x01,portid=?". The portid
+should be explicitly set.
+
+Its usage of perf::
+
+  $# perf stat -a -e fme0/fab_port_mmio_read,portid=0x0/ <command>
+  or
+  $# perf stat -a -e fme0/event=0x06,evtype=0x02,portid=0x0/ <command>
+  or
+  $# perf stat -a -e fme0/config=0x2006/ <command>
+
+Please note for fabric counters, overall perf events (fab_*) and port perf
+events (fab_port_*) actually share one set of counters in hardware, so it can't
+monitor both at the same time. If this set of counters is configured to monitor
+overall data, then per port perf data is not supported. See below example::
+
+  $# perf stat -e fme0/fab_mmio_read/,fme0/fab_port_mmio_write,\
+                                                    portid=0/ sleep 1
+
+  Performance counter stats for 'system wide':
+
+                 3      fme0/fab_mmio_read/
+   <not supported>      fme0/fab_port_mmio_write,portid=0x0/
+
+       1.001750904 seconds time elapsed
+
+The driver also provides a "cpumask" sysfs attribute, which always shows fixed
+value cpu0 as all perf events are from system-wide counters on FPGA device.
+
+The current driver does not support sampling. So "perf record" is unsupported.
+
+
 Add new FIUs support
 ====================
 It's possible that developers made some new function blocks (FIUs) under this
-- 
1.8.3.1

^ permalink raw reply related

* [RESEND Patch v6 2/2] fpga: dfl: fme: add performance reporting support
From: Wu Hao @ 2020-01-17  3:10 UTC (permalink / raw)
  To: will, mdf, mark.rutland, linux-fpga, linux-kernel
  Cc: linux-api, atull, gregkh, Wu Hao, Luwei Kang, Xu Yilun
In-Reply-To: <1579230628-22243-1-git-send-email-hao.wu@intel.com>

This patch adds support for performance reporting private feature
for FPGA Management Engine (FME). Now it supports several different
performance counters, including 'basic', 'cache', 'fabric', 'vtd'
and 'vtd_sip'. It allows user to use standard linux tools to access
these performance counters.

e.g. List all events by "perf list"

  perf list | grep fme

  fme0/cache_read_hit/                         [Kernel PMU event]
  fme0/cache_read_miss/                        [Kernel PMU event]
  ...

  fme0/fab_mmio_read/                          [Kernel PMU event]
  fme0/fab_mmio_write/                         [Kernel PMU event]
  ...

  fme0/fab_port_mmio_read,portid=?/            [Kernel PMU event]
  fme0/fab_port_mmio_write,portid=?/           [Kernel PMU event]
  ...

  fme0/vtd_port_devtlb_1g_fill,portid=?/       [Kernel PMU event]
  fme0/vtd_port_devtlb_2m_fill,portid=?/       [Kernel PMU event]
  ...

  fme0/vtd_sip_iotlb_1g_hit/                   [Kernel PMU event]
  fme0/vtd_sip_iotlb_1g_miss/                  [Kernel PMU event]
  ...

  fme0/clock                                   [Kernel PMU event]
  ...

e.g. check increased counter value after run one application using
"perf stat" command.

 perf stat -e fme0/fab_mmio_read/,fme0/fab_mmio_write/ ./test

 Performance counter stats for './test':

                 1      fme0/fab_mmio_read/
                 2      fme0/fab_mmio_write/

       1.009496520 seconds time elapsed

Please note that fabric counters support both fab_* and fab_port_*, but
actually they are sharing one set of performance counters in hardware.
If user wants to monitor overall data events on fab_* then fab_port_*
can't be supported at the same time, see example below:

perf stat -e fme0/fab_mmio_read/,fme0/fab_port_mmio_write,portid=0/

 Performance counter stats for 'system wide':

                 0      fme0/fab_mmio_read/
   <not supported>      fme0/fab_port_mmio_write,portid=0/

       2.141064085 seconds time elapsed

Signed-off-by: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
v3: replace scnprintf with sprintf in sysfs interfaces.
    update sysfs doc kernel version and date.
    fix sysfs doc issue for fabric counter.
    refine PERF_OBJ_ATTR_* macro, doesn't count on __ATTR anymore.
    introduce PERF_OBJ_ATTR_F_* macro, as it needs to use different
    filenames for some of the sysfs attributes.
    remove kobject_del when destroy kobject, kobject_put is enough.
    do sysfs_remove_groups first when destroying perf_obj.
    WARN_ON_ONCE in case internal parms are wrong in read_*_count().
v4: rework this patch to use standard perf API as user interfaces.
v5: rebase and clean up.
v6: use dev_ext_attribute instead of creating new fme_perf_attribute
    use is_visible function to decide which events to expose per
    hardware capability, and add event_init checking for all events.
---
 drivers/fpga/Makefile       |   1 +
 drivers/fpga/dfl-fme-main.c |   4 +
 drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl-fme.h      |   2 +
 drivers/fpga/dfl.h          |   2 +
 5 files changed, 952 insertions(+)
 create mode 100644 drivers/fpga/dfl-fme-perf.c

diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 4865b74..d8e21df 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_FPGA_DFL_FME_REGION)	+= dfl-fme-region.o
 obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
 dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o dfl-fme-error.o
+dfl-fme-objs += dfl-fme-perf.o
 dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
 dfl-afu-objs += dfl-afu-error.o
 
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 7c930e6..53a1447 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -580,6 +580,10 @@ static int fme_power_mgmt_init(struct platform_device *pdev,
 		.ops = &fme_power_mgmt_ops,
 	},
 	{
+		.id_table = fme_perf_id_table,
+		.ops = &fme_perf_ops,
+	},
+	{
 		.ops = NULL,
 	},
 };
diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
new file mode 100644
index 0000000..5e51727
--- /dev/null
+++ b/drivers/fpga/dfl-fme-perf.c
@@ -0,0 +1,943 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Management Engine (FME) Global Performance Reporting
+ *
+ * Copyright 2019 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Kang Luwei <luwei.kang@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xu Yilun <yilun.xu@intel.com>
+ *   Joseph Grecco <joe.grecco@intel.com>
+ *   Enno Luebbers <enno.luebbers@intel.com>
+ *   Tim Whisonant <tim.whisonant@intel.com>
+ *   Ananda Ravuri <ananda.ravuri@intel.com>
+ *   Mitchel, Henry <henry.mitchel@intel.com>
+ */
+
+#include <linux/perf_event.h>
+#include "dfl.h"
+#include "dfl-fme.h"
+
+/*
+ * Performance Counter Registers for Cache.
+ *
+ * Cache Events are listed below as CACHE_EVNT_*.
+ */
+#define CACHE_CTRL			0x8
+#define CACHE_RESET_CNTR		BIT_ULL(0)
+#define CACHE_FREEZE_CNTR		BIT_ULL(8)
+#define CACHE_CTRL_EVNT			GENMASK_ULL(19, 16)
+#define CACHE_EVNT_RD_HIT		0x0
+#define CACHE_EVNT_WR_HIT		0x1
+#define CACHE_EVNT_RD_MISS		0x2
+#define CACHE_EVNT_WR_MISS		0x3
+#define CACHE_EVNT_RSVD			0x4
+#define CACHE_EVNT_HOLD_REQ		0x5
+#define CACHE_EVNT_DATA_WR_PORT_CONTEN	0x6
+#define CACHE_EVNT_TAG_WR_PORT_CONTEN	0x7
+#define CACHE_EVNT_TX_REQ_STALL		0x8
+#define CACHE_EVNT_RX_REQ_STALL		0x9
+#define CACHE_EVNT_EVICTIONS		0xa
+#define CACHE_EVNT_MAX			CACHE_EVNT_EVICTIONS
+#define CACHE_CHANNEL_SEL		BIT_ULL(20)
+#define CACHE_CHANNEL_RD		0
+#define CACHE_CHANNEL_WR		1
+#define CACHE_CNTR0			0x10
+#define CACHE_CNTR1			0x18
+#define CACHE_CNTR_EVNT_CNTR		GENMASK_ULL(47, 0)
+#define CACHE_CNTR_EVNT			GENMASK_ULL(63, 60)
+
+/*
+ * Performance Counter Registers for Fabric.
+ *
+ * Fabric Events are listed below as FAB_EVNT_*
+ */
+#define FAB_CTRL			0x20
+#define FAB_RESET_CNTR			BIT_ULL(0)
+#define FAB_FREEZE_CNTR			BIT_ULL(8)
+#define FAB_CTRL_EVNT			GENMASK_ULL(19, 16)
+#define FAB_EVNT_PCIE0_RD		0x0
+#define FAB_EVNT_PCIE0_WR		0x1
+#define FAB_EVNT_PCIE1_RD		0x2
+#define FAB_EVNT_PCIE1_WR		0x3
+#define FAB_EVNT_UPI_RD			0x4
+#define FAB_EVNT_UPI_WR			0x5
+#define FAB_EVNT_MMIO_RD		0x6
+#define FAB_EVNT_MMIO_WR		0x7
+#define FAB_EVNT_MAX			FAB_EVNT_MMIO_WR
+#define FAB_PORT_ID			GENMASK_ULL(21, 20)
+#define FAB_PORT_FILTER			BIT_ULL(23)
+#define FAB_PORT_FILTER_DISABLE		0
+#define FAB_PORT_FILTER_ENABLE		1
+#define FAB_CNTR			0x28
+#define FAB_CNTR_EVNT_CNTR		GENMASK_ULL(59, 0)
+#define FAB_CNTR_EVNT			GENMASK_ULL(63, 60)
+
+/*
+ * Performance Counter Registers for Clock.
+ *
+ * Clock Counter can't be reset or frozen by SW.
+ */
+#define CLK_CNTR			0x30
+#define BASIC_EVNT_CLK			0x0
+#define BASIC_EVNT_MAX			BASIC_EVNT_CLK
+
+/*
+ * Performance Counter Registers for IOMMU / VT-D.
+ *
+ * VT-D Events are listed below as VTD_EVNT_* and VTD_SIP_EVNT_*
+ */
+#define VTD_CTRL			0x38
+#define VTD_RESET_CNTR			BIT_ULL(0)
+#define VTD_FREEZE_CNTR			BIT_ULL(8)
+#define VTD_CTRL_EVNT			GENMASK_ULL(19, 16)
+#define VTD_EVNT_AFU_MEM_RD_TRANS	0x0
+#define VTD_EVNT_AFU_MEM_WR_TRANS	0x1
+#define VTD_EVNT_AFU_DEVTLB_RD_HIT	0x2
+#define VTD_EVNT_AFU_DEVTLB_WR_HIT	0x3
+#define VTD_EVNT_DEVTLB_4K_FILL		0x4
+#define VTD_EVNT_DEVTLB_2M_FILL		0x5
+#define VTD_EVNT_DEVTLB_1G_FILL		0x6
+#define VTD_EVNT_MAX			VTD_EVNT_DEVTLB_1G_FILL
+#define VTD_CNTR			0x40
+#define VTD_CNTR_EVNT_CNTR		GENMASK_ULL(47, 0)
+#define VTD_CNTR_EVNT			GENMASK_ULL(63, 60)
+
+#define VTD_SIP_CTRL			0x48
+#define VTD_SIP_RESET_CNTR		BIT_ULL(0)
+#define VTD_SIP_FREEZE_CNTR		BIT_ULL(8)
+#define VTD_SIP_CTRL_EVNT		GENMASK_ULL(19, 16)
+#define VTD_SIP_EVNT_IOTLB_4K_HIT	0x0
+#define VTD_SIP_EVNT_IOTLB_2M_HIT	0x1
+#define VTD_SIP_EVNT_IOTLB_1G_HIT	0x2
+#define VTD_SIP_EVNT_SLPWC_L3_HIT	0x3
+#define VTD_SIP_EVNT_SLPWC_L4_HIT	0x4
+#define VTD_SIP_EVNT_RCC_HIT		0x5
+#define VTD_SIP_EVNT_IOTLB_4K_MISS	0x6
+#define VTD_SIP_EVNT_IOTLB_2M_MISS	0x7
+#define VTD_SIP_EVNT_IOTLB_1G_MISS	0x8
+#define VTD_SIP_EVNT_SLPWC_L3_MISS	0x9
+#define VTD_SIP_EVNT_SLPWC_L4_MISS	0xa
+#define VTD_SIP_EVNT_RCC_MISS		0xb
+#define VTD_SIP_EVNT_MAX		VTD_SIP_EVNT_SLPWC_L4_MISS
+#define VTD_SIP_CNTR			0X50
+#define VTD_SIP_CNTR_EVNT_CNTR		GENMASK_ULL(47, 0)
+#define VTD_SIP_CNTR_EVNT		GENMASK_ULL(63, 60)
+
+#define PERF_TIMEOUT			30
+
+#define PERF_MAX_PORT_NUM		1U
+
+/**
+ * struct fme_perf_priv - priv data structure for fme perf driver
+ *
+ * @dev: parent device.
+ * @ioaddr: mapped base address of mmio region.
+ * @pmu: pmu data structure for fme perf counters.
+ * @id: id of this fme performance report private feature.
+ * @fab_users: current user number on fabric counters.
+ * @fab_port_id: used to indicate current working mode of fabric counters.
+ * @fab_lock: lock to protect fabric counters working mode.
+ */
+struct fme_perf_priv {
+	struct device *dev;
+	void __iomem *ioaddr;
+	struct pmu pmu;
+	u64 id;
+
+	u32 fab_users;
+	u32 fab_port_id;
+	spinlock_t fab_lock;
+};
+
+/**
+ * struct fme_perf_event_ops - callbacks for fme perf events
+ *
+ * @event_init: callback invoked during event init.
+ * @event_destroy: callback invoked during event destroy.
+ * @read_counter: callback to read hardware counters.
+ */
+struct fme_perf_event_ops {
+	int (*event_init)(struct fme_perf_priv *priv, u32 event, u32 portid);
+	void (*event_destroy)(struct fme_perf_priv *priv, u32 event,
+			      u32 portid);
+	u64 (*read_counter)(struct fme_perf_priv *priv, u32 event, u32 portid);
+};
+
+#define to_fme_perf_priv(_pmu)	container_of(_pmu, struct fme_perf_priv, pmu)
+
+static cpumask_t fme_perf_cpumask = CPU_MASK_CPU0;
+
+static ssize_t cpumask_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	return cpumap_print_to_pagebuf(true, buf, &fme_perf_cpumask);
+}
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *fme_perf_cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group fme_perf_cpumask_group = {
+	.attrs = fme_perf_cpumask_attrs,
+};
+
+#define FME_EVENT_MASK		GENMASK_ULL(11, 0)
+#define FME_EVENT_SHIFT		0
+#define FME_EVTYPE_MASK		GENMASK_ULL(15, 12)
+#define FME_EVTYPE_SHIFT	12
+#define FME_EVTYPE_BASIC	0
+#define FME_EVTYPE_CACHE	1
+#define FME_EVTYPE_FABRIC	2
+#define FME_EVTYPE_VTD		3
+#define FME_EVTYPE_VTD_SIP	4
+#define FME_EVTYPE_MAX		FME_EVTYPE_VTD_SIP
+#define FME_PORTID_MASK		GENMASK_ULL(23, 16)
+#define FME_PORTID_SHIFT	16
+#define FME_PORTID_ROOT		(0xffU)
+
+#define get_event(_config)	FIELD_GET(FME_EVENT_MASK, _config)
+#define get_evtype(_config)	FIELD_GET(FME_EVTYPE_MASK, _config)
+#define get_portid(_config)	FIELD_GET(FME_PORTID_MASK, _config)
+
+PMU_FORMAT_ATTR(event,		"config:0-11");
+PMU_FORMAT_ATTR(evtype,		"config:12-15");
+PMU_FORMAT_ATTR(portid,		"config:16-23");
+
+static struct attribute *fme_perf_format_attrs[] = {
+	&format_attr_event.attr,
+	&format_attr_evtype.attr,
+	&format_attr_portid.attr,
+	NULL,
+};
+
+static struct attribute_group fme_perf_format_group = {
+	.name = "format",
+	.attrs = fme_perf_format_attrs,
+};
+
+static struct attribute *fme_perf_events_attrs_empty[] = {
+	NULL,
+};
+
+static struct attribute_group fme_perf_events_group = {
+	.name = "events",
+	.attrs = fme_perf_events_attrs_empty,
+};
+
+static const struct attribute_group *fme_perf_groups[] = {
+	&fme_perf_format_group,
+	&fme_perf_cpumask_group,
+	&fme_perf_events_group,
+	NULL,
+};
+
+static bool is_portid_root(u32 portid)
+{
+	return portid == FME_PORTID_ROOT;
+}
+
+static bool is_portid_port(u32 portid)
+{
+	return portid < PERF_MAX_PORT_NUM;
+}
+
+static bool is_portid_root_or_port(u32 portid)
+{
+	return is_portid_root(portid) || is_portid_port(portid);
+}
+
+static int basic_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
+{
+	if (event <= BASIC_EVNT_MAX && is_portid_root(portid))
+		return 0;
+
+	return -EINVAL;
+}
+
+static u64 basic_read_event_counter(struct fme_perf_priv *priv,
+				    u32 event, u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+
+	return readq(base + CLK_CNTR);
+}
+
+static int cache_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
+{
+	if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
+	    event <= CACHE_EVNT_MAX && is_portid_root(portid))
+		return 0;
+
+	return -EINVAL;
+}
+
+static u64 cache_read_event_counter(struct fme_perf_priv *priv,
+				    u32 event, u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+	u64 v, count;
+	u8 channel;
+
+	if (event == CACHE_EVNT_WR_HIT || event == CACHE_EVNT_WR_MISS ||
+	    event == CACHE_EVNT_DATA_WR_PORT_CONTEN ||
+	    event == CACHE_EVNT_TAG_WR_PORT_CONTEN)
+		channel = CACHE_CHANNEL_WR;
+	else
+		channel = CACHE_CHANNEL_RD;
+
+	/* set channel access type and cache event code. */
+	v = readq(base + CACHE_CTRL);
+	v &= ~(CACHE_CHANNEL_SEL | CACHE_CTRL_EVNT);
+	v |= FIELD_PREP(CACHE_CHANNEL_SEL, channel);
+	v |= FIELD_PREP(CACHE_CTRL_EVNT, event);
+	writeq(v, base + CACHE_CTRL);
+
+	if (readq_poll_timeout_atomic(base + CACHE_CNTR0, v,
+				      FIELD_GET(CACHE_CNTR_EVNT, v) == event,
+				      1, PERF_TIMEOUT)) {
+		dev_err(priv->dev, "timeout, unmatched cache event code in counter register.\n");
+		return 0;
+	}
+
+	v = readq(base + CACHE_CNTR0);
+	count = FIELD_GET(CACHE_CNTR_EVNT_CNTR, v);
+	v = readq(base + CACHE_CNTR1);
+	count += FIELD_GET(CACHE_CNTR_EVNT_CNTR, v);
+
+	return count;
+}
+
+static bool is_fabric_event_supported(struct fme_perf_priv *priv, u32 event,
+				      u32 portid)
+{
+	if (event > FAB_EVNT_MAX || !is_portid_root_or_port(portid))
+		return false;
+
+	if (priv->id == FME_FEATURE_ID_GLOBAL_DPERF &&
+	    (event == FAB_EVNT_PCIE1_RD || event == FAB_EVNT_UPI_RD ||
+	     event == FAB_EVNT_PCIE1_WR || event == FAB_EVNT_UPI_WR))
+		return false;
+
+	return true;
+}
+
+static int fabric_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+	int ret = 0;
+	u64 v;
+
+	if (!is_fabric_event_supported(priv, event, portid))
+		return -EINVAL;
+
+	/*
+	 * as fabric counter set only can be in either overall or port mode.
+	 * In overall mode, it counts overall data for FPGA, and in port mode,
+	 * it is configured to monitor on one individual port.
+	 *
+	 * so every time, a new event is initialized, driver checks
+	 * current working mode and if someone is using this counter set.
+	 */
+	spin_lock(&priv->fab_lock);
+	if (priv->fab_users && priv->fab_port_id != portid) {
+		dev_dbg(priv->dev, "conflict fabric event monitoring mode.\n");
+		ret = -EOPNOTSUPP;
+		goto exit;
+	}
+
+	priv->fab_users++;
+
+	/*
+	 * skip if current working mode matches, otherwise change the working
+	 * mode per input port_id, to monitor overall data or another port.
+	 */
+	if (priv->fab_port_id == portid)
+		goto exit;
+
+	priv->fab_port_id = portid;
+
+	v = readq(base + FAB_CTRL);
+	v &= ~(FAB_PORT_FILTER | FAB_PORT_ID);
+
+	if (is_portid_root(portid)) {
+		v |= FIELD_PREP(FAB_PORT_FILTER, FAB_PORT_FILTER_DISABLE);
+	} else {
+		v |= FIELD_PREP(FAB_PORT_FILTER, FAB_PORT_FILTER_ENABLE);
+		v |= FIELD_PREP(FAB_PORT_ID, portid);
+	}
+	writeq(v, base + FAB_CTRL);
+
+exit:
+	spin_unlock(&priv->fab_lock);
+	return ret;
+}
+
+static void fabric_event_destroy(struct fme_perf_priv *priv, u32 event,
+				 u32 portid)
+{
+	spin_lock(&priv->fab_lock);
+	priv->fab_users--;
+	spin_unlock(&priv->fab_lock);
+}
+
+static u64 fabric_read_event_counter(struct fme_perf_priv *priv, u32 event,
+				     u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+	u64 v;
+
+	v = readq(base + FAB_CTRL);
+	v &= ~FAB_CTRL_EVNT;
+	v |= FIELD_PREP(FAB_CTRL_EVNT, event);
+	writeq(v, base + FAB_CTRL);
+
+	if (readq_poll_timeout_atomic(base + FAB_CNTR, v,
+				      FIELD_GET(FAB_CNTR_EVNT, v) == event,
+				      1, PERF_TIMEOUT)) {
+		dev_err(priv->dev, "timeout, unmatched fab event code in counter register.\n");
+		return 0;
+	}
+
+	v = readq(base + FAB_CNTR);
+	return FIELD_GET(FAB_CNTR_EVNT_CNTR, v);
+}
+
+static int vtd_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
+{
+	if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
+	    event <= VTD_EVNT_MAX && is_portid_port(portid))
+		return 0;
+
+	return -EINVAL;
+}
+
+static u64 vtd_read_event_counter(struct fme_perf_priv *priv, u32 event,
+				  u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+	u64 v;
+
+	event += (portid * (VTD_EVNT_MAX + 1));
+
+	v = readq(base + VTD_CTRL);
+	v &= ~VTD_CTRL_EVNT;
+	v |= FIELD_PREP(VTD_CTRL_EVNT, event);
+	writeq(v, base + VTD_CTRL);
+
+	if (readq_poll_timeout_atomic(base + VTD_CNTR, v,
+				      FIELD_GET(VTD_CNTR_EVNT, v) == event,
+				      1, PERF_TIMEOUT)) {
+		dev_err(priv->dev, "timeout, unmatched vtd event code in counter register.\n");
+		return 0;
+	}
+
+	v = readq(base + VTD_CNTR);
+	return FIELD_GET(VTD_CNTR_EVNT_CNTR, v);
+}
+
+static int vtd_sip_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
+{
+	if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
+	    event <= VTD_SIP_EVNT_MAX && is_portid_root(portid))
+		return 0;
+
+	return -EINVAL;
+}
+
+static u64 vtd_sip_read_event_counter(struct fme_perf_priv *priv, u32 event,
+				      u32 portid)
+{
+	void __iomem *base = priv->ioaddr;
+	u64 v;
+
+	v = readq(base + VTD_SIP_CTRL);
+	v &= ~VTD_SIP_CTRL_EVNT;
+	v |= FIELD_PREP(VTD_SIP_CTRL_EVNT, event);
+	writeq(v, base + VTD_SIP_CTRL);
+
+	if (readq_poll_timeout_atomic(base + VTD_SIP_CNTR, v,
+				      FIELD_GET(VTD_SIP_CNTR_EVNT, v) == event,
+				      1, PERF_TIMEOUT)) {
+		dev_err(priv->dev, "timeout, unmatched vtd sip event code in counter register\n");
+		return 0;
+	}
+
+	v = readq(base + VTD_SIP_CNTR);
+	return FIELD_GET(VTD_SIP_CNTR_EVNT_CNTR, v);
+}
+
+static struct fme_perf_event_ops fme_perf_event_ops[] = {
+	[FME_EVTYPE_BASIC]	= {.event_init = basic_event_init,
+				   .read_counter = basic_read_event_counter,},
+	[FME_EVTYPE_CACHE]	= {.event_init = cache_event_init,
+				   .read_counter = cache_read_event_counter,},
+	[FME_EVTYPE_FABRIC]	= {.event_init = fabric_event_init,
+				   .event_destroy = fabric_event_destroy,
+				   .read_counter = fabric_read_event_counter,},
+	[FME_EVTYPE_VTD]	= {.event_init = vtd_event_init,
+				   .read_counter = vtd_read_event_counter,},
+	[FME_EVTYPE_VTD_SIP]	= {.event_init = vtd_sip_event_init,
+				   .read_counter = vtd_sip_read_event_counter,},
+};
+
+static ssize_t fme_perf_event_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct dev_ext_attribute *eattr;
+	unsigned long config;
+	char *ptr = buf;
+
+	eattr = container_of(attr, struct dev_ext_attribute, attr);
+	config = (unsigned long)eattr->var;
+
+	ptr += sprintf(ptr, "event=0x%02x", (unsigned int)get_event(config));
+	ptr += sprintf(ptr, ",evtype=0x%02x", (unsigned int)get_evtype(config));
+
+	if (is_portid_root(get_portid(config)))
+		ptr += sprintf(ptr, ",portid=0x%02x\n", FME_PORTID_ROOT);
+	else
+		ptr += sprintf(ptr, ",portid=?\n");
+
+	return (ssize_t)(ptr - buf);
+}
+
+#define FME_EVENT_ATTR(_name) \
+	__ATTR(_name, 0444, fme_perf_event_show, NULL)
+
+#define FME_PORT_EVENT_CONFIG(_event, _type)				\
+	(void *)((((_event) << FME_EVENT_SHIFT) & FME_EVENT_MASK) |	\
+		(((_type) << FME_EVTYPE_SHIFT) & FME_EVTYPE_MASK))
+
+#define FME_EVENT_CONFIG(_event, _type)					\
+	(void *)((((_event) << FME_EVENT_SHIFT) & FME_EVENT_MASK) |	\
+		(((_type) << FME_EVTYPE_SHIFT) & FME_EVTYPE_MASK) |	\
+		(FME_PORTID_ROOT << FME_PORTID_SHIFT))
+
+/* FME Perf Basic Events */
+#define FME_EVENT_BASIC(_name, _event)					\
+static struct dev_ext_attribute fme_perf_event_##_name = {		\
+	.attr = FME_EVENT_ATTR(_name),					\
+	.var = FME_EVENT_CONFIG(_event, FME_EVTYPE_BASIC),		\
+}
+
+FME_EVENT_BASIC(clock, BASIC_EVNT_CLK);
+
+static struct attribute *fme_perf_basic_events_attrs[] = {
+	&fme_perf_event_clock.attr.attr,
+	NULL,
+};
+
+static const struct attribute_group fme_perf_basic_events_group = {
+	.name = "events",
+	.attrs = fme_perf_basic_events_attrs,
+};
+
+/* FME Perf Fabric Events */
+#define FME_EVENT_FABRIC(_name, _event)					\
+static struct dev_ext_attribute fme_perf_event_fab_##_name = {		\
+	.attr = FME_EVENT_ATTR(fab_##_name),				\
+	.var = FME_EVENT_CONFIG(_event, FME_EVTYPE_FABRIC),		\
+}
+
+#define FME_EVENT_FABRIC_PORT(_name, _event)				\
+static struct dev_ext_attribute fme_perf_event_fab_port_##_name = {	\
+	.attr = FME_EVENT_ATTR(fab_port_##_name),			\
+	.var = FME_PORT_EVENT_CONFIG(_event, FME_EVTYPE_FABRIC),	\
+}
+
+FME_EVENT_FABRIC(pcie0_read,  FAB_EVNT_PCIE0_RD);
+FME_EVENT_FABRIC(pcie0_write, FAB_EVNT_PCIE0_WR);
+FME_EVENT_FABRIC(pcie1_read,  FAB_EVNT_PCIE1_RD);
+FME_EVENT_FABRIC(pcie1_write, FAB_EVNT_PCIE1_WR);
+FME_EVENT_FABRIC(upi_read,    FAB_EVNT_UPI_RD);
+FME_EVENT_FABRIC(upi_write,   FAB_EVNT_UPI_WR);
+FME_EVENT_FABRIC(mmio_read,   FAB_EVNT_MMIO_RD);
+FME_EVENT_FABRIC(mmio_write,  FAB_EVNT_MMIO_WR);
+
+FME_EVENT_FABRIC_PORT(pcie0_read,  FAB_EVNT_PCIE0_RD);
+FME_EVENT_FABRIC_PORT(pcie0_write, FAB_EVNT_PCIE0_WR);
+FME_EVENT_FABRIC_PORT(pcie1_read,  FAB_EVNT_PCIE1_RD);
+FME_EVENT_FABRIC_PORT(pcie1_write, FAB_EVNT_PCIE1_WR);
+FME_EVENT_FABRIC_PORT(upi_read,    FAB_EVNT_UPI_RD);
+FME_EVENT_FABRIC_PORT(upi_write,   FAB_EVNT_UPI_WR);
+FME_EVENT_FABRIC_PORT(mmio_read,   FAB_EVNT_MMIO_RD);
+FME_EVENT_FABRIC_PORT(mmio_write,  FAB_EVNT_MMIO_WR);
+
+static struct attribute *fme_perf_fabric_events_attrs[] = {
+	&fme_perf_event_fab_pcie0_read.attr.attr,
+	&fme_perf_event_fab_pcie0_write.attr.attr,
+	&fme_perf_event_fab_pcie1_read.attr.attr,
+	&fme_perf_event_fab_pcie1_write.attr.attr,
+	&fme_perf_event_fab_upi_read.attr.attr,
+	&fme_perf_event_fab_upi_write.attr.attr,
+	&fme_perf_event_fab_mmio_read.attr.attr,
+	&fme_perf_event_fab_mmio_write.attr.attr,
+	&fme_perf_event_fab_port_pcie0_read.attr.attr,
+	&fme_perf_event_fab_port_pcie0_write.attr.attr,
+	&fme_perf_event_fab_port_pcie1_read.attr.attr,
+	&fme_perf_event_fab_port_pcie1_write.attr.attr,
+	&fme_perf_event_fab_port_upi_read.attr.attr,
+	&fme_perf_event_fab_port_upi_write.attr.attr,
+	&fme_perf_event_fab_port_mmio_read.attr.attr,
+	&fme_perf_event_fab_port_mmio_write.attr.attr,
+	NULL,
+};
+
+static umode_t fme_perf_fabric_events_visible(struct kobject *kobj,
+					      struct attribute *attr, int n)
+{
+	struct pmu *pmu = dev_get_drvdata(kobj_to_dev(kobj));
+	struct fme_perf_priv *priv = to_fme_perf_priv(pmu);
+	struct dev_ext_attribute *eattr;
+	u64 var;
+
+	eattr = container_of(attr, struct dev_ext_attribute, attr.attr);
+	var = (u64)eattr->var;
+
+	if (is_fabric_event_supported(priv, get_event(var), get_portid(var)))
+		return attr->mode;
+
+	return 0;
+}
+
+static const struct attribute_group fme_perf_fabric_events_group = {
+	.name = "events",
+	.attrs = fme_perf_fabric_events_attrs,
+	.is_visible = fme_perf_fabric_events_visible,
+};
+
+/* FME Perf Cache Events */
+#define FME_EVENT_CACHE(_name, _event)					\
+static struct dev_ext_attribute fme_perf_event_cache_##_name = {	\
+	.attr = FME_EVENT_ATTR(cache_##_name),				\
+	.var = FME_EVENT_CONFIG(_event, FME_EVTYPE_CACHE),		\
+}
+
+FME_EVENT_CACHE(read_hit,     CACHE_EVNT_RD_HIT);
+FME_EVENT_CACHE(read_miss,    CACHE_EVNT_RD_MISS);
+FME_EVENT_CACHE(write_hit,    CACHE_EVNT_WR_HIT);
+FME_EVENT_CACHE(write_miss,   CACHE_EVNT_WR_MISS);
+FME_EVENT_CACHE(hold_request, CACHE_EVNT_HOLD_REQ);
+FME_EVENT_CACHE(tx_req_stall, CACHE_EVNT_TX_REQ_STALL);
+FME_EVENT_CACHE(rx_req_stall, CACHE_EVNT_RX_REQ_STALL);
+FME_EVENT_CACHE(eviction,     CACHE_EVNT_EVICTIONS);
+FME_EVENT_CACHE(data_write_port_contention, CACHE_EVNT_DATA_WR_PORT_CONTEN);
+FME_EVENT_CACHE(tag_write_port_contention,  CACHE_EVNT_TAG_WR_PORT_CONTEN);
+
+static struct attribute *fme_perf_cache_events_attrs[] = {
+	&fme_perf_event_cache_read_hit.attr.attr,
+	&fme_perf_event_cache_read_miss.attr.attr,
+	&fme_perf_event_cache_write_hit.attr.attr,
+	&fme_perf_event_cache_write_miss.attr.attr,
+	&fme_perf_event_cache_hold_request.attr.attr,
+	&fme_perf_event_cache_tx_req_stall.attr.attr,
+	&fme_perf_event_cache_rx_req_stall.attr.attr,
+	&fme_perf_event_cache_eviction.attr.attr,
+	&fme_perf_event_cache_data_write_port_contention.attr.attr,
+	&fme_perf_event_cache_tag_write_port_contention.attr.attr,
+	NULL,
+};
+
+static umode_t fme_perf_events_visible(struct kobject *kobj,
+				       struct attribute *attr, int n)
+{
+	struct pmu *pmu = dev_get_drvdata(kobj_to_dev(kobj));
+	struct fme_perf_priv *priv = to_fme_perf_priv(pmu);
+
+	return (priv->id == FME_FEATURE_ID_GLOBAL_IPERF) ? attr->mode : 0;
+}
+
+static const struct attribute_group fme_perf_cache_events_group = {
+	.name = "events",
+	.attrs = fme_perf_cache_events_attrs,
+	.is_visible = fme_perf_events_visible,
+};
+
+/* FME Perf VTD Events */
+#define FME_EVENT_VTD_PORT(_name, _event)				\
+static struct dev_ext_attribute fme_perf_event_vtd_port_##_name = {	\
+	.attr = FME_EVENT_ATTR(vtd_port_##_name),			\
+	.var = FME_PORT_EVENT_CONFIG(_event, FME_EVTYPE_VTD),		\
+}
+
+FME_EVENT_VTD_PORT(read_transaction,  VTD_EVNT_AFU_MEM_RD_TRANS);
+FME_EVENT_VTD_PORT(write_transaction, VTD_EVNT_AFU_MEM_WR_TRANS);
+FME_EVENT_VTD_PORT(devtlb_read_hit,   VTD_EVNT_AFU_DEVTLB_RD_HIT);
+FME_EVENT_VTD_PORT(devtlb_write_hit,  VTD_EVNT_AFU_DEVTLB_WR_HIT);
+FME_EVENT_VTD_PORT(devtlb_4k_fill,    VTD_EVNT_DEVTLB_4K_FILL);
+FME_EVENT_VTD_PORT(devtlb_2m_fill,    VTD_EVNT_DEVTLB_2M_FILL);
+FME_EVENT_VTD_PORT(devtlb_1g_fill,    VTD_EVNT_DEVTLB_1G_FILL);
+
+static struct attribute *fme_perf_vtd_events_attrs[] = {
+	&fme_perf_event_vtd_port_read_transaction.attr.attr,
+	&fme_perf_event_vtd_port_write_transaction.attr.attr,
+	&fme_perf_event_vtd_port_devtlb_read_hit.attr.attr,
+	&fme_perf_event_vtd_port_devtlb_write_hit.attr.attr,
+	&fme_perf_event_vtd_port_devtlb_4k_fill.attr.attr,
+	&fme_perf_event_vtd_port_devtlb_2m_fill.attr.attr,
+	&fme_perf_event_vtd_port_devtlb_1g_fill.attr.attr,
+	NULL,
+};
+
+static const struct attribute_group fme_perf_vtd_events_group = {
+	.name = "events",
+	.attrs = fme_perf_vtd_events_attrs,
+	.is_visible = fme_perf_events_visible,
+};
+
+/* FME Perf VTD SIP Events */
+#define FME_EVENT_VTD_SIP(_name, _event)				\
+static struct dev_ext_attribute fme_perf_event_vtd_sip_##_name = {	\
+	.attr = FME_EVENT_ATTR(vtd_sip_##_name),			\
+	.var = FME_EVENT_CONFIG(_event, FME_EVTYPE_VTD_SIP),		\
+}
+
+FME_EVENT_VTD_SIP(iotlb_4k_hit,  VTD_SIP_EVNT_IOTLB_4K_HIT);
+FME_EVENT_VTD_SIP(iotlb_2m_hit,  VTD_SIP_EVNT_IOTLB_2M_HIT);
+FME_EVENT_VTD_SIP(iotlb_1g_hit,  VTD_SIP_EVNT_IOTLB_1G_HIT);
+FME_EVENT_VTD_SIP(slpwc_l3_hit,  VTD_SIP_EVNT_SLPWC_L3_HIT);
+FME_EVENT_VTD_SIP(slpwc_l4_hit,  VTD_SIP_EVNT_SLPWC_L4_HIT);
+FME_EVENT_VTD_SIP(rcc_hit,       VTD_SIP_EVNT_RCC_HIT);
+FME_EVENT_VTD_SIP(iotlb_4k_miss, VTD_SIP_EVNT_IOTLB_4K_MISS);
+FME_EVENT_VTD_SIP(iotlb_2m_miss, VTD_SIP_EVNT_IOTLB_2M_MISS);
+FME_EVENT_VTD_SIP(iotlb_1g_miss, VTD_SIP_EVNT_IOTLB_1G_MISS);
+FME_EVENT_VTD_SIP(slpwc_l3_miss, VTD_SIP_EVNT_SLPWC_L3_MISS);
+FME_EVENT_VTD_SIP(slpwc_l4_miss, VTD_SIP_EVNT_SLPWC_L4_MISS);
+FME_EVENT_VTD_SIP(rcc_miss,      VTD_SIP_EVNT_RCC_MISS);
+
+static struct attribute *fme_perf_vtd_sip_events_attrs[] = {
+	&fme_perf_event_vtd_sip_iotlb_4k_hit.attr.attr,
+	&fme_perf_event_vtd_sip_iotlb_2m_hit.attr.attr,
+	&fme_perf_event_vtd_sip_iotlb_1g_hit.attr.attr,
+	&fme_perf_event_vtd_sip_slpwc_l3_hit.attr.attr,
+	&fme_perf_event_vtd_sip_slpwc_l4_hit.attr.attr,
+	&fme_perf_event_vtd_sip_rcc_hit.attr.attr,
+	&fme_perf_event_vtd_sip_iotlb_4k_miss.attr.attr,
+	&fme_perf_event_vtd_sip_iotlb_2m_miss.attr.attr,
+	&fme_perf_event_vtd_sip_iotlb_1g_miss.attr.attr,
+	&fme_perf_event_vtd_sip_slpwc_l3_miss.attr.attr,
+	&fme_perf_event_vtd_sip_slpwc_l4_miss.attr.attr,
+	&fme_perf_event_vtd_sip_rcc_miss.attr.attr,
+	NULL,
+};
+
+static const struct attribute_group fme_perf_vtd_sip_events_group = {
+	.name = "events",
+	.attrs = fme_perf_vtd_sip_events_attrs,
+	.is_visible = fme_perf_events_visible,
+};
+
+static const struct attribute_group *fme_perf_events_groups[] = {
+	&fme_perf_basic_events_group,
+	&fme_perf_cache_events_group,
+	&fme_perf_fabric_events_group,
+	&fme_perf_vtd_events_group,
+	&fme_perf_vtd_sip_events_group,
+	NULL,
+};
+
+static struct fme_perf_event_ops *get_event_ops(u32 evtype)
+{
+	if (evtype > FME_EVTYPE_MAX)
+		return NULL;
+
+	return &fme_perf_event_ops[evtype];
+}
+
+static void fme_perf_event_destroy(struct perf_event *event)
+{
+	struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
+	struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
+
+	if (ops->event_destroy)
+		ops->event_destroy(priv, event->hw.idx, event->hw.config_base);
+}
+
+static int fme_perf_event_init(struct perf_event *event)
+{
+	struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	struct fme_perf_event_ops *ops;
+	u32 eventid, evtype, portid;
+
+	/* test the event attr type check for PMU enumeration */
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/*
+	 * fme counters are shared across all cores.
+	 * Therefore, it does not support per-process mode.
+	 * Also, it does not support event sampling mode.
+	 */
+	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+		return -EINVAL;
+
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	eventid = get_event(event->attr.config);
+	portid = get_portid(event->attr.config);
+	evtype = get_evtype(event->attr.config);
+	if (evtype > FME_EVTYPE_MAX)
+		return -EINVAL;
+
+	hwc->event_base = evtype;
+	hwc->idx = (int)eventid;
+	hwc->config_base = portid;
+
+	event->destroy = fme_perf_event_destroy;
+
+	dev_dbg(priv->dev, "%s event=0x%x, evtype=0x%x, portid=0x%x,\n",
+		__func__, eventid, evtype, portid);
+
+	ops = get_event_ops(evtype);
+	if (ops->event_init)
+		return ops->event_init(priv, eventid, portid);
+
+	return 0;
+}
+
+static void fme_perf_event_update(struct perf_event *event)
+{
+	struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
+	struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 now, prev, delta;
+
+	now = ops->read_counter(priv, (u32)hwc->idx, hwc->config_base);
+	prev = local64_read(&hwc->prev_count);
+	delta = now - prev;
+
+	local64_add(delta, &event->count);
+}
+
+static void fme_perf_event_start(struct perf_event *event, int flags)
+{
+	struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
+	struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 count;
+
+	count = ops->read_counter(priv, (u32)hwc->idx, hwc->config_base);
+	local64_set(&hwc->prev_count, count);
+}
+
+static void fme_perf_event_stop(struct perf_event *event, int flags)
+{
+	fme_perf_event_update(event);
+}
+
+static int fme_perf_event_add(struct perf_event *event, int flags)
+{
+	if (flags & PERF_EF_START)
+		fme_perf_event_start(event, flags);
+
+	return 0;
+}
+
+static void fme_perf_event_del(struct perf_event *event, int flags)
+{
+	fme_perf_event_stop(event, PERF_EF_UPDATE);
+}
+
+static void fme_perf_event_read(struct perf_event *event)
+{
+	fme_perf_event_update(event);
+}
+
+static void fme_perf_setup_hardware(struct fme_perf_priv *priv)
+{
+	void __iomem *base = priv->ioaddr;
+	u64 v;
+
+	/* read and save current working mode for fabric counters */
+	v = readq(base + FAB_CTRL);
+
+	if (FIELD_GET(FAB_PORT_FILTER, v) == FAB_PORT_FILTER_DISABLE)
+		priv->fab_port_id = FME_PORTID_ROOT;
+	else
+		priv->fab_port_id = FIELD_GET(FAB_PORT_ID, v);
+}
+
+static int fme_perf_pmu_register(struct platform_device *pdev,
+				 struct fme_perf_priv *priv)
+{
+	struct pmu *pmu = &priv->pmu;
+	char *name;
+	int ret;
+
+	spin_lock_init(&priv->fab_lock);
+
+	fme_perf_setup_hardware(priv);
+
+	pmu->task_ctx_nr =	perf_invalid_context;
+	pmu->attr_groups =	fme_perf_groups;
+	pmu->attr_update =	fme_perf_events_groups;
+	pmu->event_init =	fme_perf_event_init;
+	pmu->add =		fme_perf_event_add;
+	pmu->del =		fme_perf_event_del;
+	pmu->start =		fme_perf_event_start;
+	pmu->stop =		fme_perf_event_stop;
+	pmu->read =		fme_perf_event_read;
+	pmu->capabilities =	PERF_PMU_CAP_NO_INTERRUPT |
+				PERF_PMU_CAP_NO_EXCLUDE;
+
+	name = devm_kasprintf(priv->dev, GFP_KERNEL, "fme%d", pdev->id);
+
+	ret = perf_pmu_register(pmu, name, -1);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void fme_perf_pmu_unregister(struct fme_perf_priv *priv)
+{
+	perf_pmu_unregister(&priv->pmu);
+}
+
+static int fme_perf_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
+{
+	struct fme_perf_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->ioaddr = feature->ioaddr;
+	priv->id = feature->id;
+
+	ret = fme_perf_pmu_register(pdev, priv);
+	if (ret)
+		return ret;
+
+	feature->priv = priv;
+	return 0;
+}
+
+static void fme_perf_uinit(struct platform_device *pdev,
+			   struct dfl_feature *feature)
+{
+	struct fme_perf_priv *priv = feature->priv;
+
+	fme_perf_pmu_unregister(priv);
+}
+
+const struct dfl_feature_id fme_perf_id_table[] = {
+	{.id = FME_FEATURE_ID_GLOBAL_IPERF,},
+	{.id = FME_FEATURE_ID_GLOBAL_DPERF,},
+	{0,}
+};
+
+const struct dfl_feature_ops fme_perf_ops = {
+	.init = fme_perf_init,
+	.uinit = fme_perf_uinit,
+};
diff --git a/drivers/fpga/dfl-fme.h b/drivers/fpga/dfl-fme.h
index 6685c8e..4195dd6 100644
--- a/drivers/fpga/dfl-fme.h
+++ b/drivers/fpga/dfl-fme.h
@@ -38,5 +38,7 @@ struct dfl_fme {
 extern const struct dfl_feature_ops fme_global_err_ops;
 extern const struct dfl_feature_id fme_global_err_id_table[];
 extern const struct attribute_group fme_global_err_group;
+extern const struct dfl_feature_ops fme_perf_ops;
+extern const struct dfl_feature_id fme_perf_id_table[];
 
 #endif /* __DFL_FME_H */
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 9f0e656..d312678 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -197,12 +197,14 @@ struct dfl_feature_driver {
  *		    feature dev (platform device)'s reources.
  * @ioaddr: mapped mmio resource address.
  * @ops: ops of this sub feature.
+ * @priv: priv data of this feature.
  */
 struct dfl_feature {
 	u64 id;
 	int resource_index;
 	void __iomem *ioaddr;
 	const struct dfl_feature_ops *ops;
+	void *priv;
 };
 
 #define DEV_STATUS_IN_USE	0
-- 
1.8.3.1

^ permalink raw reply related

* Re: [RESEND Patch v6 0/2] add performance reporting support to FPGA DFL drivers
From: Wu Hao @ 2020-01-17  3:39 UTC (permalink / raw)
  To: will-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mdf-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-fpga-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, atull-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <1579230628-22243-1-git-send-email-hao.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Hi Will

I resend this patch per suggestion from Greg.

Actually this is one important feature for users, but was blocked by
code review for quite a long time (no comments since last November).

We really need your help on code review from perf, please help us..

Many Thanks!

Hao

On Fri, Jan 17, 2020 at 11:10:26AM +0800, Wu Hao wrote:
> Hi all,
> 
> This patchset adds performance reporting support for FPGA DFL drivers. It
> introduces one pmu to expose userspace interfaces via standard perf API.
> User could use standard perf tool to access perf events exposed via pmu.
> 
> This patchset is generated based on latest fpga/for-next branch.
> 
> Main changes from v5:
>  - use dev_ext_attribute instead of fme_perf_event_attr.
>  - use is_visible function to decide which events to expose per
>    hardware capability, and add event_init checking for all events.
> 
> Main changes from v4:
>  - rebase and clean up.
>  - update Kconfig for PERF_EVENTS dependency.
> 
> Main changes from v3:
>  - add more descriptions in doc, including how to use perf tool for these
>    hardware counters. (patch #1)
>  - use standard perf API instead of sysfs entries. (patch #2)
> 
> Wu Hao (1):
>   fpga: dfl: fme: add performance reporting support
> 
> Xu Yilun (1):
>   Documentation: fpga: dfl: add description for performance reporting
>     support
> 
>  Documentation/fpga/dfl.rst  |  83 ++++
>  drivers/fpga/Makefile       |   1 +
>  drivers/fpga/dfl-fme-main.c |   4 +
>  drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl-fme.h      |   2 +
>  drivers/fpga/dfl.h          |   2 +
>  6 files changed, 1035 insertions(+)
>  create mode 100644 drivers/fpga/dfl-fme-perf.c
> 
> -- 
> 1.8.3.1

^ permalink raw reply

* Re: [PATCH] fscrypt: reserve flags for hardware-wrapped keys feature
From: Christoph Hellwig @ 2020-01-17  8:12 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt-u79uwXL29TY76Z2rM5mHXA,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA,
	linux-f2fs-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Barani Muthukumaran,
	Gaurav Kashyap, Theodore Ts'o, Jaegeuk Kim,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200116192008.35766-1-ebiggers-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Thu, Jan 16, 2020 at 11:20:08AM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> 
> Reserve flags for the hardware-wrapped keys feature which is being
> worked on [1].  FSCRYPT_POLICY_FLAG_HW_WRAPPED_KEY will denote that the
> encryption policy needs a hardware-wrapped key to be unlocked.
> FSCRYPT_ADD_KEY_FLAG_HW_WRAPPED will denote that the key being added is
> a hardware-wrapped key.
> 
> This reservation is tentative, and these codepoints may be reused if the
> feature is not upstreamed.

NAK.  While the feature itself sounds really useful we don't just
reserve format bits for code not upstream.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox