From: Thomas Gleixner <tglx@linutronix.de>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: fenghua.yu@intel.com, tony.luck@intel.com,
gavin.hindman@intel.com, vikas.shivappa@linux.intel.com,
dave.hansen@intel.com, mingo@redhat.com, hpa@zytor.com,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH V2 06/22] x86/intel_rdt: Create pseudo-locked regions
Date: Mon, 19 Feb 2018 21:57:52 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.21.1802192138140.1853@nanos.tec.linutronix.de> (raw)
In-Reply-To: <17cceeb077f3ac5f50b110285b36905091a345b0.1518443616.git.reinette.chatre@intel.com>
On Tue, 13 Feb 2018, Reinette Chatre wrote:
> System administrator creates/removes pseudo-locked regions by
> creating/removing directories in the pseudo-lock subdirectory of the
> resctrl filesystem. Here we add directory creation and removal support.
>
> A "pseudo-lock region" is introduced, which represents an
> instance of a pseudo-locked cache region. During mkdir a new region is
> created but since we do not know which cache it belongs to at that time
> we maintain a global pointer to it from where it will be moved to the cache
> (rdt_domain) it belongs to after initialization. This implies that
> we only support one uninitialized pseudo-locked region at a time.
Whats the reason for this restriction? If there are uninitialized
directories, so what?
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> arch/x86/kernel/cpu/intel_rdt.h | 3 +
> arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 220 +++++++++++++++++++++++++++-
> 2 files changed, 222 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
> index 8f5ded384e19..55f085985072 100644
> --- a/arch/x86/kernel/cpu/intel_rdt.h
> +++ b/arch/x86/kernel/cpu/intel_rdt.h
> @@ -352,6 +352,7 @@ extern struct mutex rdtgroup_mutex;
> extern struct rdt_resource rdt_resources_all[];
> extern struct rdtgroup rdtgroup_default;
> DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
> +extern struct kernfs_node *pseudo_lock_kn;
>
> int __init rdtgroup_init(void);
>
> @@ -457,5 +458,7 @@ bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
> void __check_limbo(struct rdt_domain *d, bool force_free);
> int rdt_pseudo_lock_fs_init(struct kernfs_node *root);
> void rdt_pseudo_lock_fs_remove(void);
> +int rdt_pseudo_lock_mkdir(const char *name, umode_t mode);
> +int rdt_pseudo_lock_rmdir(struct kernfs_node *kn);
>
> #endif /* _ASM_X86_INTEL_RDT_H */
> diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
> index a787a103c432..7a22e367b82f 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
> @@ -20,11 +20,142 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/kernfs.h>
> +#include <linux/kref.h>
> #include <linux/seq_file.h>
> #include <linux/stat.h>
> +#include <linux/slab.h>
> #include "intel_rdt.h"
>
> -static struct kernfs_node *pseudo_lock_kn;
> +struct kernfs_node *pseudo_lock_kn;
> +
> +/*
> + * Protect the pseudo_lock_region access. Since we will link to
> + * pseudo_lock_region from rdt domains rdtgroup_mutex should be obtained
> + * first if needed.
> + */
> +static DEFINE_MUTEX(rdt_pseudo_lock_mutex);
> +
> +/**
> + * struct pseudo_lock_region - pseudo-lock region information
> + * @kn: kernfs node representing this region in the resctrl
> + * filesystem
> + * @cbm: bitmask of the pseudo-locked region
> + * @cpu: core associated with the cache on which the setup code
> + * will be run
> + * @minor: minor number of character device associated with this
> + * region
> + * @locked: state indicating if this region has been locked or not
> + * @refcount: how many are waiting to access this pseudo-lock
> + * region via kernfs
> + * @deleted: user requested removal of region via rmdir on kernfs
> + */
> +struct pseudo_lock_region {
> + struct kernfs_node *kn;
> + u32 cbm;
> + int cpu;
> + unsigned int minor;
> + bool locked;
> + struct kref refcount;
> + bool deleted;
> +};
> +
> +/*
> + * Only one uninitialized pseudo-locked region can exist at a time. An
> + * uninitialized pseudo-locked region is created when the user creates a
> + * new directory within the pseudo_lock subdirectory of the resctrl
> + * filsystem. The user will initialize the pseudo-locked region by writing
> + * to its schemata file at which point this structure will be moved to the
> + * cache domain it belongs to.
> + */
> +static struct pseudo_lock_region *new_plr;
Why isn't the struct pointer not stored in the corresponding kernfs's node->priv?
> +static void __pseudo_lock_region_release(struct pseudo_lock_region *plr)
> +{
> + bool is_new_plr = (plr == new_plr);
> +
> + WARN_ON(!plr->deleted);
> + if (!plr->deleted)
> + return;
if (WARN_ON(...))
return;
> +
> + kfree(plr);
> + if (is_new_plr)
> + new_plr = NULL;
> +}
> +
> +static void pseudo_lock_region_release(struct kref *ref)
> +{
> + struct pseudo_lock_region *plr = container_of(ref,
> + struct pseudo_lock_region,
> + refcount);
You simply can avoid those line breaks by:
struct pseudo_lock_region *plr;
plr = container_of(ref, struct pseudo_lock_region, refcount);
Hmm?
> + mutex_lock(&rdt_pseudo_lock_mutex);
> + __pseudo_lock_region_release(plr);
> + mutex_unlock(&rdt_pseudo_lock_mutex);
> +}
> +
> +/**
> + * pseudo_lock_region_kn_lock - Obtain lock to pseudo-lock region kernfs node
> + *
> + * This is called from the kernfs related functions which are called with
> + * an active reference to the kernfs_node that contains a valid pointer to
> + * the pseudo-lock region it represents. We can thus safely take an active
> + * reference to the pseudo-lock region before dropping the reference to the
> + * kernfs_node.
> + *
> + * We need to handle the scenarios where the kernfs directory representing
> + * this pseudo-lock region can be removed while an application still has an
> + * open handle to one of the directory's files and operations on this
> + * handle are attempted.
> + * To support this we allow a file operation to drop its reference to the
> + * kernfs_node so that the removal can proceed, while using the mutex to
> + * ensure these operations on the pseudo-lock region are serialized. At the
> + * time an operation does obtain access to the region it may thus have been
> + * deleted.
> + */
> +static struct pseudo_lock_region *pseudo_lock_region_kn_lock(
> + struct kernfs_node *kn)
> +{
> + struct pseudo_lock_region *plr = (kernfs_type(kn) == KERNFS_DIR) ?
> + kn->priv : kn->parent->priv;
See above.
> +int rdt_pseudo_lock_mkdir(const char *name, umode_t mode)
> +{
> + struct pseudo_lock_region *plr;
> + struct kernfs_node *kn;
> + int ret = 0;
> +
> + mutex_lock(&rdtgroup_mutex);
> + mutex_lock(&rdt_pseudo_lock_mutex);
> +
> + if (new_plr) {
> + ret = -ENOSPC;
> + goto out;
> + }
> +
> + plr = kzalloc(sizeof(*plr), GFP_KERNEL);
> + if (!plr) {
> + ret = -ENOSPC;
ENOMEM is the proper error code here.
> + goto out;
> + }
> +
> + kn = kernfs_create_dir(pseudo_lock_kn, name, mode, plr);
> + if (IS_ERR(kn)) {
> + ret = PTR_ERR(kn);
> + goto out_free;
> + }
> +
> + plr->kn = kn;
> + ret = rdtgroup_kn_set_ugid(kn);
> + if (ret)
> + goto out_remove;
> +
> + kref_init(&plr->refcount);
> + kernfs_activate(kn);
> + new_plr = plr;
> + ret = 0;
> + goto out;
> +
> +out_remove:
> + kernfs_remove(kn);
> +out_free:
> + kfree(plr);
> +out:
> + mutex_unlock(&rdt_pseudo_lock_mutex);
> + mutex_unlock(&rdtgroup_mutex);
> + return ret;
> +}
> +
> +/*
> + * rdt_pseudo_lock_rmdir - Remove pseudo-lock region
> + *
> + * LOCKING:
> + * Since the pseudo-locked region can be associated with a RDT domain at
> + * removal we take both rdtgroup_mutex and rdt_pseudo_lock_mutex to protect
> + * the rdt_domain access as well as the pseudo_lock_region access.
Is there a real reason / benefit for having this second mutex?
Thanks,
tglx
next prev parent reply other threads:[~2018-02-19 20:57 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-13 15:46 [RFC PATCH V2 00/22] Intel(R) Resource Director Technology Cache Pseudo-Locking enabling Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 01/22] x86/intel_rdt: Documentation for Cache Pseudo-Locking Reinette Chatre
2018-02-19 20:35 ` Thomas Gleixner
2018-02-19 22:15 ` Reinette Chatre
2018-02-19 22:19 ` Thomas Gleixner
2018-02-19 22:24 ` Reinette Chatre
2018-02-19 21:27 ` Randy Dunlap
2018-02-19 22:21 ` Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 02/22] x86/intel_rdt: Make useful functions available internally Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 03/22] x86/intel_rdt: Introduce hooks to create pseudo-locking files Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 04/22] x86/intel_rdt: Introduce test to determine if closid is in use Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 05/22] x86/intel_rdt: Print more accurate pseudo-locking availability Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 06/22] x86/intel_rdt: Create pseudo-locked regions Reinette Chatre
2018-02-19 20:57 ` Thomas Gleixner [this message]
2018-02-19 23:02 ` Reinette Chatre
2018-02-19 23:16 ` Thomas Gleixner
2018-02-20 3:21 ` Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 07/22] x86/intel_rdt: Connect pseudo-locking directory to operations Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 08/22] x86/intel_rdt: Introduce pseudo-locking resctrl files Reinette Chatre
2018-02-19 21:01 ` Thomas Gleixner
2018-02-13 15:46 ` [RFC PATCH V2 09/22] x86/intel_rdt: Discover supported platforms via prefetch disable bits Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 10/22] x86/intel_rdt: Disable pseudo-locking if CDP enabled Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 11/22] x86/intel_rdt: Associate pseudo-locked regions with its domain Reinette Chatre
2018-02-19 21:19 ` Thomas Gleixner
2018-02-19 23:00 ` Reinette Chatre
2018-02-19 23:19 ` Thomas Gleixner
2018-02-20 3:17 ` Reinette Chatre
2018-02-20 10:00 ` Thomas Gleixner
2018-02-20 16:02 ` Reinette Chatre
2018-02-20 17:18 ` Thomas Gleixner
2018-02-13 15:46 ` [RFC PATCH V2 12/22] x86/intel_rdt: Support CBM checking from value and character buffer Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 13/22] x86/intel_rdt: Support schemata write - pseudo-locking core Reinette Chatre
2018-02-20 17:15 ` Thomas Gleixner
2018-02-20 18:47 ` Reinette Chatre
2018-02-20 23:21 ` Thomas Gleixner
2018-02-21 1:58 ` Mike Kravetz
2018-02-21 6:10 ` Reinette Chatre
2018-02-21 8:34 ` Thomas Gleixner
2018-02-21 5:58 ` Reinette Chatre
2018-02-27 0:34 ` Reinette Chatre
2018-02-27 10:36 ` Thomas Gleixner
2018-02-27 15:38 ` Thomas Gleixner
2018-02-27 19:52 ` Reinette Chatre
2018-02-27 21:33 ` Reinette Chatre
2018-02-28 18:39 ` Thomas Gleixner
2018-02-28 19:17 ` Reinette Chatre
2018-02-28 19:40 ` Thomas Gleixner
2018-02-27 21:01 ` Reinette Chatre
2018-02-28 17:57 ` Thomas Gleixner
2018-02-28 17:59 ` Thomas Gleixner
2018-02-28 18:34 ` Reinette Chatre
2018-02-28 18:42 ` Thomas Gleixner
2018-02-13 15:46 ` [RFC PATCH V2 14/22] x86/intel_rdt: Enable testing for pseudo-locked region Reinette Chatre
2018-02-13 15:46 ` [RFC PATCH V2 15/22] x86/intel_rdt: Prevent new allocations from pseudo-locked regions Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 16/22] x86/intel_rdt: Create debugfs files for pseudo-locking testing Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 17/22] x86/intel_rdt: Create character device exposing pseudo-locked region Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 18/22] x86/intel_rdt: More precise L2 hit/miss measurements Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 19/22] x86/intel_rdt: Support L3 cache performance event of Broadwell Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 20/22] x86/intel_rdt: Limit C-states dynamically when pseudo-locking active Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 21/22] mm/hugetlb: Enable large allocations through gigantic page API Reinette Chatre
2018-02-13 15:47 ` [RFC PATCH V2 22/22] x86/intel_rdt: Support contiguous memory of all sizes Reinette Chatre
2018-02-14 18:12 ` [RFC PATCH V2 00/22] Intel(R) Resource Director Technology Cache Pseudo-Locking enabling Mike Kravetz
2018-02-14 18:31 ` Reinette Chatre
2018-02-15 20:39 ` Reinette Chatre
2018-02-15 21:10 ` Mike Kravetz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1802192138140.1853@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=dave.hansen@intel.com \
--cc=fenghua.yu@intel.com \
--cc=gavin.hindman@intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=reinette.chatre@intel.com \
--cc=tony.luck@intel.com \
--cc=vikas.shivappa@linux.intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).