diff mbox

[RFC,v2,3/7] cgroup: Add interface to allow drivers to lookup process cgroup membership

Message ID 20180201195315.4956-4-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Roper Feb. 1, 2018, 7:53 p.m. UTC
Drivers will need to save/restore the appropriate cgroup data for the process
submitting a driver request.  Add an interface for drivers to determine the
cgroup for the userspace process submitting a driver request.

Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 include/linux/cgroup.h        |  6 ++++++
 kernel/cgroup/cgroup_driver.c | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

Comments

Chris Wilson Feb. 1, 2018, 8:49 p.m. UTC | #1
Quoting Matt Roper (2018-02-01 19:53:11)
> Drivers will need to save/restore the appropriate cgroup data for the process
> submitting a driver request.  Add an interface for drivers to determine the
> cgroup for the userspace process submitting a driver request.
> 
> Cc: Tejun Heo <tj@kernel.org>
> Cc: cgroups@vger.kernel.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  include/linux/cgroup.h        |  6 ++++++
>  kernel/cgroup/cgroup_driver.c | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 0ba1374122c7..05ebfb97bcde 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -842,6 +842,7 @@ void cgroup_driver_release(struct cgroup_driver *drv);
>  struct cgroup_driver_data * cgroup_driver_get_data(struct cgroup_driver *drv,
>                                                    struct cgroup *cgrp,
>                                                    bool *is_new);
> +struct cgroup *cgroup_for_driver_process(struct pid *pid);
>  
>  #else /* !CONFIG_CGROUP_DRIVER */
>  
> @@ -857,6 +858,11 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
>  {
>         return ERR_PTR(-EINVAL);
>  }
> +static inline struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)
> +{
> +       return NULL;
> +}
>  
>  #endif /* !CONFIG_CGROUP_DRIVER */
>  
> diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
> index 0d893395dc7b..4f870cbb9212 100644
> --- a/kernel/cgroup/cgroup_driver.c
> +++ b/kernel/cgroup/cgroup_driver.c
> @@ -128,3 +128,27 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
>         return data;
>  }
>  EXPORT_SYMBOL(cgroup_driver_get_data);
> +
> +/**
> + * cgroup_for_driver_process - return the cgroup for a process
> + * @pid: process to lookup cgroup for
> + *
> + * Returns the cgroup from the v2 hierarchy that a process belongs to.
> + * This function is intended to be called from drivers and will obtain
> + * the necessary cgroup locks.
> + *
> + * RETURNS:
> + * Process' cgroup in the default (v2) hierarchy
> + */
> +struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)
> +{
> +       struct task_struct *task = pid_task(pid, PIDTYPE_PID);
> +
> +       mutex_lock(&cgroup_mutex);
> +       spin_lock_irq(&css_set_lock);
> +       task_cgroup_from_root(task, &cgrp_dfl_root);
> +       spin_unlock_irq(&css_set_lock);
> +       mutex_unlock(&cgroup_mutex);

Gut feeling says use task = get_pid_task(); if (!task) return NULL and
put_task_struct(task); for safety.

Shouldn't there be a return here?
> +}
-Chris
Matt Roper Feb. 1, 2018, 9:25 p.m. UTC | #2
On Thu, Feb 01, 2018 at 08:49:32PM +0000, Chris Wilson wrote:
> Quoting Matt Roper (2018-02-01 19:53:11)
...
> > +struct cgroup *
> > +cgroup_for_driver_process(struct pid *pid)
> > +{
> > +       struct task_struct *task = pid_task(pid, PIDTYPE_PID);
> > +
> > +       mutex_lock(&cgroup_mutex);
> > +       spin_lock_irq(&css_set_lock);
> > +       task_cgroup_from_root(task, &cgrp_dfl_root);
> > +       spin_unlock_irq(&css_set_lock);
> > +       mutex_unlock(&cgroup_mutex);
> 
> Gut feeling says use task = get_pid_task(); if (!task) return NULL and
> put_task_struct(task); for safety.
> 
> Shouldn't there be a return here?

Woops, looks like I mis-squashed some of my fixes for this patch in to
patch #4 instead.  But yeah, get_pid_task() is better in general.


Matt

> > +}
> -Chris
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Feb. 7, 2018, 10:42 p.m. UTC | #3
Hello,

On Thu, Feb 01, 2018 at 11:53:11AM -0800, Matt Roper wrote:
> +/**
> + * cgroup_for_driver_process - return the cgroup for a process
> + * @pid: process to lookup cgroup for
> + *
> + * Returns the cgroup from the v2 hierarchy that a process belongs to.
> + * This function is intended to be called from drivers and will obtain
> + * the necessary cgroup locks.
> + *
> + * RETURNS:
> + * Process' cgroup in the default (v2) hierarchy
> + */
> +struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)

I think you probably want to use task_dfl_cgroup().  We can add
task_get_dfl_cgroup() in a similar fashion to task_get_css() if
necessary.

Thanks.
diff mbox

Patch

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 0ba1374122c7..05ebfb97bcde 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -842,6 +842,7 @@  void cgroup_driver_release(struct cgroup_driver *drv);
 struct cgroup_driver_data * cgroup_driver_get_data(struct cgroup_driver *drv,
 						   struct cgroup *cgrp,
 						   bool *is_new);
+struct cgroup *cgroup_for_driver_process(struct pid *pid);
 
 #else /* !CONFIG_CGROUP_DRIVER */
 
@@ -857,6 +858,11 @@  cgroup_driver_get_data(struct cgroup_driver *drv,
 {
 	return ERR_PTR(-EINVAL);
 }
+static inline struct cgroup *
+cgroup_for_driver_process(struct pid *pid)
+{
+	return NULL;
+}
 
 #endif /* !CONFIG_CGROUP_DRIVER */
 
diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
index 0d893395dc7b..4f870cbb9212 100644
--- a/kernel/cgroup/cgroup_driver.c
+++ b/kernel/cgroup/cgroup_driver.c
@@ -128,3 +128,27 @@  cgroup_driver_get_data(struct cgroup_driver *drv,
 	return data;
 }
 EXPORT_SYMBOL(cgroup_driver_get_data);
+
+/**
+ * cgroup_for_driver_process - return the cgroup for a process
+ * @pid: process to lookup cgroup for
+ *
+ * Returns the cgroup from the v2 hierarchy that a process belongs to.
+ * This function is intended to be called from drivers and will obtain
+ * the necessary cgroup locks.
+ *
+ * RETURNS:
+ * Process' cgroup in the default (v2) hierarchy
+ */
+struct cgroup *
+cgroup_for_driver_process(struct pid *pid)
+{
+	struct task_struct *task = pid_task(pid, PIDTYPE_PID);
+
+	mutex_lock(&cgroup_mutex);
+	spin_lock_irq(&css_set_lock);
+	task_cgroup_from_root(task, &cgrp_dfl_root);
+	spin_unlock_irq(&css_set_lock);
+	mutex_unlock(&cgroup_mutex);
+}
+EXPORT_SYMBOL(cgroup_for_driver_process);