Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1612045 > unrolled thread

[PATCH] x86/intel_rdt: Add cpus_list rdtgroup file

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@kernel.org> - 2017-03-29 17:20 +0200
    Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Fenghua Yu <fenghua.yu@intel.com> - 2017-03-29 18:10 +0200
      Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-03-29 18:20 +0200
        Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-03-30 17:10 +0200
          Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Fenghua Yu <fenghua.yu@intel.com> - 2017-04-01 04:00 +0200
      Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Thomas Gleixner <tglx@linutronix.de> - 2017-03-31 10:50 +0200
        Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Fenghua Yu <fenghua.yu@intel.com> - 2017-03-31 18:00 +0200
      Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Thomas Gleixner <tglx@linutronix.de> - 2017-03-31 11:20 +0200
        Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Fenghua Yu <fenghua.yu@intel.com> - 2017-03-31 17:40 +0200
        Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-03-31 18:00 +0200
          [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-04-09 18:50 +0200
            Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file Fenghua Yu <fenghua.yu@intel.com> - 2017-04-09 19:40 +0200
              [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-04-10 09:50 +0200
                Re: [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 11:50 +0200
                  [PATCHv4] x86/intel_rdt: Add cpus_list rdtgroup file Jiri Olsa <jolsa@redhat.com> - 2017-04-10 17:00 +0200
              Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 11:20 +0200

#1612045 — [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file

While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.

Adding cpus_list file to provide cpumask list interface
to define group's cpus.

  # cd /sys/fs/resctrl/
  # echo 1-10 > krava/cpus_list
  # cat krava/cpus_list
  1-10
  # cat krava/cpus
  0007fe
  # cat cpus
  fffff9
  # cat cpus_list
  0,3-23

Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/include/asm/intel_rdt.h         | 16 +++++++++---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 42 +++++++++++++++++++++++---------
 arch/x86/kernel/cpu/intel_rdt_schemata.c |  6 +++--
 3 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 0d64397cee58..0e74e58e1f23 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
 /* rdtgroup.flags */
 #define	RDT_DELETED		1
 
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST	1
+
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
 
@@ -54,16 +57,19 @@ struct rftype {
 	char			*name;
 	umode_t			mode;
 	struct kernfs_ops	*kf_ops;
+	unsigned long		flags;
 
 	int (*seq_show)(struct kernfs_open_file *of,
-			struct seq_file *sf, void *v);
+			struct seq_file *sf, void *v,
+			unsigned long flags);
 	/*
 	 * write() is the generic write callback which maps directly to
 	 * kernfs write operation and overrides all other operations.
 	 * Maximum write size is determined by ->max_write_len.
 	 */
 	ssize_t (*write)(struct kernfs_open_file *of,
-			 char *buf, size_t nbytes, loff_t off);
+			 char *buf, size_t nbytes, loff_t off,
+			 unsigned long flags);
 };
 
 /**
@@ -179,9 +185,11 @@ void rdt_cbm_update(void *arg);
 struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn);
 void rdtgroup_kn_unlock(struct kernfs_node *kn);
 ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
-				char *buf, size_t nbytes, loff_t off);
+				char *buf, size_t nbytes, loff_t off,
+				unsigned long flags);
 int rdtgroup_schemata_show(struct kernfs_open_file *of,
-			   struct seq_file *s, void *v);
+			   struct seq_file *s, void *v,
+			   unsigned long flags);
 
 /*
  * intel_rdt_sched_in() - Writes the task's CLOSid to IA32_PQR_MSR
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..3a1dfb421dc5 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -153,7 +153,7 @@ static int rdtgroup_seqfile_show(struct seq_file *m, void *arg)
 	struct rftype *rft = of->kn->priv;
 
 	if (rft->seq_show)
-		return rft->seq_show(of, m, arg);
+		return rft->seq_show(of, m, arg, rft->flags);
 	return 0;
 }
 
@@ -163,7 +163,7 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
 	struct rftype *rft = of->kn->priv;
 
 	if (rft->write)
-		return rft->write(of, buf, nbytes, off);
+		return rft->write(of, buf, nbytes, off, rft->flags);
 
 	return -EINVAL;
 }
@@ -175,15 +175,17 @@ static struct kernfs_ops rdtgroup_kf_single_ops = {
 };
 
 static int rdtgroup_cpus_show(struct kernfs_open_file *of,
-			      struct seq_file *s, void *v)
+			      struct seq_file *s, void *v,
+			      unsigned long flags)
 {
+	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
 	struct rdtgroup *rdtgrp;
 	int ret = 0;
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 
 	if (rdtgrp)
-		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
+		seq_printf(s, fmt, cpumask_pr_args(&rdtgrp->cpu_mask));
 	else
 		ret = -ENOENT;
 	rdtgroup_kn_unlock(of->kn);
@@ -230,7 +232,8 @@ rdt_update_closid(const struct cpumask *cpu_mask, int *closid)
 }
 
 static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
-				   char *buf, size_t nbytes, loff_t off)
+				   char *buf, size_t nbytes, loff_t off,
+				   unsigned long flags)
 {
 	cpumask_var_t tmpmask, newmask;
 	struct rdtgroup *rdtgrp, *r;
@@ -252,7 +255,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		goto unlock;
 	}
 
-	ret = cpumask_parse(buf, newmask);
+	if (flags & RFTYPE_FLAGS_CPUS_LIST)
+		ret = cpulist_parse(buf, newmask);
+	else
+		ret = cpumask_parse(buf, newmask);
+
 	if (ret)
 		goto unlock;
 
@@ -415,7 +422,8 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
 }
 
 static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
-				    char *buf, size_t nbytes, loff_t off)
+				    char *buf, size_t nbytes, loff_t off,
+				    unsigned long flags __maybe_unused)
 {
 	struct rdtgroup *rdtgrp;
 	int ret = 0;
@@ -448,7 +456,8 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
 }
 
 static int rdtgroup_tasks_show(struct kernfs_open_file *of,
-			       struct seq_file *s, void *v)
+			       struct seq_file *s, void *v,
+			       unsigned long flags)
 {
 	struct rdtgroup *rdtgrp;
 	int ret = 0;
@@ -473,6 +482,14 @@ static struct rftype rdtgroup_base_files[] = {
 		.seq_show	= rdtgroup_cpus_show,
 	},
 	{
+		.name		= "cpus_list",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.write		= rdtgroup_cpus_write,
+		.seq_show	= rdtgroup_cpus_show,
+		.flags		= RFTYPE_FLAGS_CPUS_LIST,
+	},
+	{
 		.name		= "tasks",
 		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
@@ -489,7 +506,8 @@ static struct rftype rdtgroup_base_files[] = {
 };
 
 static int rdt_num_closids_show(struct kernfs_open_file *of,
-				struct seq_file *seq, void *v)
+				struct seq_file *seq, void *v,
+				unsigned long flags __maybe_unused)
 {
 	struct rdt_resource *r = of->kn->parent->priv;
 
@@ -499,7 +517,8 @@ static int rdt_num_closids_show(struct kernfs_open_file *of,
 }
 
 static int rdt_cbm_mask_show(struct kernfs_open_file *of,
-			     struct seq_file *seq, void *v)
+			     struct seq_file *seq, void *v,
+			     unsigned long flags __maybe_unused)
 {
 	struct rdt_resource *r = of->kn->parent->priv;
 
@@ -509,7 +528,8 @@ static int rdt_cbm_mask_show(struct kernfs_open_file *of,
 }
 
 static int rdt_min_cbm_bits_show(struct kernfs_open_file *of,
-			     struct seq_file *seq, void *v)
+				 struct seq_file *seq, void *v,
+				 unsigned long flags __maybe_unused)
 {
 	struct rdt_resource *r = of->kn->parent->priv;
 
diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c
index f369cb8db0d5..3780ebebf36c 100644
--- a/arch/x86/kernel/cpu/intel_rdt_schemata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c
@@ -132,7 +132,8 @@ static int update_domains(struct rdt_resource *r, int closid)
 }
 
 ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
-				char *buf, size_t nbytes, loff_t off)
+				char *buf, size_t nbytes, loff_t off,
+				unsigned long flags)
 {
 	struct rdtgroup *rdtgrp;
 	struct rdt_resource *r;
@@ -224,7 +225,8 @@ static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
 }
 
 int rdtgroup_schemata_show(struct kernfs_open_file *of,
-			   struct seq_file *s, void *v)
+			   struct seq_file *s, void *v,
+			   unsigned long flags __maybe_unused)
 {
 	struct rdtgroup *rdtgrp;
 	struct rdt_resource *r;
-- 
2.9.3

[toc] | [next] | [standalone]

#1612106

On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> While playing with the resctrl interface I found it much
> easier to deal with cpumask list rather than just regular
> cpumask.

Could you please explain specifically why and when it's easier
to deal with cpumask list? In programming cases, cpumask
and cpumask list are almost same. And people are working
on higher level tools to control resctrl. The tools can
hide detailed regular cpumask or cpumask list and user
doesn't need to care lower level format of cpumask. So
is it really useful to add cpus_list?

> 
> Adding cpus_list file to provide cpumask list interface
> to define group's cpus.
> 
>   # cd /sys/fs/resctrl/
>   # echo 1-10 > krava/cpus_list
>   # cat krava/cpus_list
>   1-10
>   # cat krava/cpus
>   0007fe
>   # cat cpus
>   fffff9
>   # cat cpus_list
>   0,3-23
> 
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  arch/x86/include/asm/intel_rdt.h         | 16 +++++++++---
>  arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 42 +++++++++++++++++++++++---------
>  arch/x86/kernel/cpu/intel_rdt_schemata.c |  6 +++--
>  3 files changed, 47 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
> index 0d64397cee58..0e74e58e1f23 100644
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -37,6 +37,9 @@ struct rdtgroup {
>  /* rdtgroup.flags */
>  #define	RDT_DELETED		1
>  
> +/* rftype.flags */
> +#define RFTYPE_FLAGS_CPUS_LIST	1
> +
>  /* List of all resource groups */
>  extern struct list_head rdt_all_groups;
>  
> @@ -54,16 +57,19 @@ struct rftype {
>  	char			*name;
>  	umode_t			mode;
>  	struct kernfs_ops	*kf_ops;
> +	unsigned long		flags;
>  
>  	int (*seq_show)(struct kernfs_open_file *of,
> -			struct seq_file *sf, void *v);
> +			struct seq_file *sf, void *v,
> +			unsigned long flags);
>  	/*
>  	 * write() is the generic write callback which maps directly to
>  	 * kernfs write operation and overrides all other operations.
>  	 * Maximum write size is determined by ->max_write_len.
>  	 */
>  	ssize_t (*write)(struct kernfs_open_file *of,
> -			 char *buf, size_t nbytes, loff_t off);
> +			 char *buf, size_t nbytes, loff_t off,
> +			 unsigned long flags);

I think no need to use "flags". Withou adding this "flags", the patch
will be much concise.

>  };
>  
>  /**
> @@ -179,9 +185,11 @@ void rdt_cbm_update(void *arg);
>  struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn);
>  void rdtgroup_kn_unlock(struct kernfs_node *kn);
>  ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
> -				char *buf, size_t nbytes, loff_t off);
> +				char *buf, size_t nbytes, loff_t off,
> +				unsigned long flags);
>  int rdtgroup_schemata_show(struct kernfs_open_file *of,
> -			   struct seq_file *s, void *v);
> +			   struct seq_file *s, void *v,
> +			   unsigned long flags);

No need for this hunk.

>  
>  /*
>   * intel_rdt_sched_in() - Writes the task's CLOSid to IA32_PQR_MSR
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 9ac2a5cdd9c2..3a1dfb421dc5 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -153,7 +153,7 @@ static int rdtgroup_seqfile_show(struct seq_file *m, void *arg)
>  	struct rftype *rft = of->kn->priv;
>  
>  	if (rft->seq_show)
> -		return rft->seq_show(of, m, arg);
> +		return rft->seq_show(of, m, arg, rft->flags);
>  }

Ditto.

>  
> @@ -163,7 +163,7 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
>  	struct rftype *rft = of->kn->priv;
>  
>  	if (rft->write)
> -		return rft->write(of, buf, nbytes, off);
> +		return rft->write(of, buf, nbytes, off, rft->flags);
>  
>  	return -EINVAL;
>  }

Ditto.

> @@ -175,15 +175,17 @@ static struct kernfs_ops rdtgroup_kf_single_ops = {
>  };
>  
>  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> -			      struct seq_file *s, void *v)
> +			      struct seq_file *s, void *v,
> +			      unsigned long flags)
>  {
> +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";

Change to:
+ const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";

>  	struct rdtgroup *rdtgrp;
>  	int ret = 0;
>  
>  	rdtgrp = rdtgroup_kn_lock_live(of->kn);
>  
>  	if (rdtgrp)
> -		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> +		seq_printf(s, fmt, cpumask_pr_args(&rdtgrp->cpu_mask));
>  	else
>  		ret = -ENOENT;
>  	rdtgroup_kn_unlock(of->kn);
> @@ -230,7 +232,8 @@ rdt_update_closid(const struct cpumask *cpu_mask, int *closid)
>  }
>  
>  static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
> -				   char *buf, size_t nbytes, loff_t off)
> +				   char *buf, size_t nbytes, loff_t off,
> +				   unsigned long flags)
>  {
>  	cpumask_var_t tmpmask, newmask;
>  	struct rdtgroup *rdtgrp, *r;

No need for this hunk.

> @@ -252,7 +255,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>  		goto unlock;
>  	}
>  
> -	ret = cpumask_parse(buf, newmask);
> +	if (flags & RFTYPE_FLAGS_CPUS_LIST)

Change to:
+	if (strcmp(of->kn->priv, "cpus"))

> +		ret = cpulist_parse(buf, newmask);
> +	else
> +		ret = cpumask_parse(buf, newmask);
> +
>  	if (ret)
>  		goto unlock;
>  
> @@ -415,7 +422,8 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
>  }
>  
>  static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
> -				    char *buf, size_t nbytes, loff_t off)
> +				    char *buf, size_t nbytes, loff_t off,
> +				    unsigned long flags __maybe_unused)
>  {
>  	struct rdtgroup *rdtgrp;
>  	int ret = 0;

No need for this hunk.

> @@ -448,7 +456,8 @@ static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
>  }
>  
>  static int rdtgroup_tasks_show(struct kernfs_open_file *of,
> -			       struct seq_file *s, void *v)
> +			       struct seq_file *s, void *v,
> +			       unsigned long flags)
>  {
>  	struct rdtgroup *rdtgrp;
>  	int ret = 0;

Ditto.

> @@ -473,6 +482,14 @@ static struct rftype rdtgroup_base_files[] = {
>  		.seq_show	= rdtgroup_cpus_show,
>  	},
>  	{
> +		.name		= "cpus_list",
> +		.mode		= 0644,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.write		= rdtgroup_cpus_write,
> +		.seq_show	= rdtgroup_cpus_show,
> +		.flags		= RFTYPE_FLAGS_CPUS_LIST,
> +	},
> +	{
>  		.name		= "tasks",
>  		.mode		= 0644,
>  		.kf_ops		= &rdtgroup_kf_single_ops,

No need for the rest of the patch.

> @@ -489,7 +506,8 @@ static struct rftype rdtgroup_base_files[] = {
>  };
>  
>  static int rdt_num_closids_show(struct kernfs_open_file *of,
> -				struct seq_file *seq, void *v)
> +				struct seq_file *seq, void *v,
> +				unsigned long flags __maybe_unused)
>  {
>  	struct rdt_resource *r = of->kn->parent->priv;
>  
> @@ -499,7 +517,8 @@ static int rdt_num_closids_show(struct kernfs_open_file *of,
>  }
>  
>  static int rdt_cbm_mask_show(struct kernfs_open_file *of,
> -			     struct seq_file *seq, void *v)
> +			     struct seq_file *seq, void *v,
> +			     unsigned long flags __maybe_unused)
>  {
>  	struct rdt_resource *r = of->kn->parent->priv;
>  
> @@ -509,7 +528,8 @@ static int rdt_cbm_mask_show(struct kernfs_open_file *of,
>  }
>  
>  static int rdt_min_cbm_bits_show(struct kernfs_open_file *of,
> -			     struct seq_file *seq, void *v)
> +				 struct seq_file *seq, void *v,
> +				 unsigned long flags __maybe_unused)
>  {
>  	struct rdt_resource *r = of->kn->parent->priv;
>  
> diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c
> index f369cb8db0d5..3780ebebf36c 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c
> @@ -132,7 +132,8 @@ static int update_domains(struct rdt_resource *r, int closid)
>  }
>  
>  ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
> -				char *buf, size_t nbytes, loff_t off)
> +				char *buf, size_t nbytes, loff_t off,
> +				unsigned long flags)
>  {
>  	struct rdtgroup *rdtgrp;
>  	struct rdt_resource *r;
> @@ -224,7 +225,8 @@ static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
>  }
>  
>  int rdtgroup_schemata_show(struct kernfs_open_file *of,
> -			   struct seq_file *s, void *v)
> +			   struct seq_file *s, void *v,
> +			   unsigned long flags __maybe_unused)
>  {
>  	struct rdtgroup *rdtgrp;
>  	struct rdt_resource *r;
> -- 
> 2.9.3
> 

[toc] | [prev] | [next] | [standalone]

#1612117

On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > While playing with the resctrl interface I found it much
> > easier to deal with cpumask list rather than just regular
> > cpumask.
> 
> Could you please explain specifically why and when it's easier
> to deal with cpumask list? In programming cases, cpumask
> and cpumask list are almost same. And people are working
> on higher level tools to control resctrl. The tools can
> hide detailed regular cpumask or cpumask list and user
> doesn't need to care lower level format of cpumask. So
> is it really useful to add cpus_list?


well I'm not aware about any such tool so I used resctrl
interface directly, and in that case it was much simpler

jirka

[toc] | [prev] | [next] | [standalone]

#1613154

On Wed, Mar 29, 2017 at 06:16:00PM +0200, Jiri Olsa wrote:
> On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > While playing with the resctrl interface I found it much
> > > easier to deal with cpumask list rather than just regular
> > > cpumask.
> > 
> > Could you please explain specifically why and when it's easier
> > to deal with cpumask list? In programming cases, cpumask
> > and cpumask list are almost same. And people are working
> > on higher level tools to control resctrl. The tools can
> > hide detailed regular cpumask or cpumask list and user
> > doesn't need to care lower level format of cpumask. So
> > is it really useful to add cpus_list?
> 
> 
> well I'm not aware about any such tool so I used resctrl
> interface directly, and in that case it was much simpler

is there any tool available for this actualy?

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1614353

On Thu, Mar 30, 2017 at 04:59:47PM +0200, Jiri Olsa wrote:
> On Wed, Mar 29, 2017 at 06:16:00PM +0200, Jiri Olsa wrote:
> > On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> > > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > > While playing with the resctrl interface I found it much
> > > > easier to deal with cpumask list rather than just regular
> > > > cpumask.
> > > 
> > > Could you please explain specifically why and when it's easier
> > > to deal with cpumask list? In programming cases, cpumask
> > > and cpumask list are almost same. And people are working
> > > on higher level tools to control resctrl. The tools can
> > > hide detailed regular cpumask or cpumask list and user
> > > doesn't need to care lower level format of cpumask. So
> > > is it really useful to add cpus_list?
> > 
> > 
> > well I'm not aware about any such tool so I used resctrl
> > interface directly, and in that case it was much simpler
> 
> is there any tool available for this actualy?

Here is a tool created by Marcelo:
https://lwn.net/Articles/710514/

Thanks.

-Fenghua

[toc] | [prev] | [next] | [standalone]

#1613750

On Wed, 29 Mar 2017, Fenghua Yu wrote:
> On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > While playing with the resctrl interface I found it much
> > easier to deal with cpumask list rather than just regular
> > cpumask.
> 
> Could you please explain specifically why and when it's easier
> to deal with cpumask list? In programming cases, cpumask
> and cpumask list are almost same. And people are working
> on higher level tools to control resctrl. The tools can
> hide detailed regular cpumask or cpumask list and user
> doesn't need to care lower level format of cpumask. So
> is it really useful to add cpus_list?

Yes, because a lot of people including me do not care about these tools at
all. Making it easy to read and write from the command line is the first
thing to do.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1614129

On Fri, Mar 31, 2017 at 10:47:52AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > While playing with the resctrl interface I found it much
> > > easier to deal with cpumask list rather than just regular
> > > cpumask.
> > 
> > Could you please explain specifically why and when it's easier
> > to deal with cpumask list? In programming cases, cpumask
> > and cpumask list are almost same. And people are working
> > on higher level tools to control resctrl. The tools can
> > hide detailed regular cpumask or cpumask list and user
> > doesn't need to care lower level format of cpumask. So
> > is it really useful to add cpus_list?
> 
> Yes, because a lot of people including me do not care about these tools at
> all. Making it easy to read and write from the command line is the first
> thing to do.

Sure, you are right. Hopefully adding a bit more explanation in
the commit message will be helpful.

Thanks.

-Fenghua

[toc] | [prev] | [next] | [standalone]

#1613790

On Wed, 29 Mar 2017, Fenghua Yu wrote:
> >  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > -			      struct seq_file *s, void *v)
> > +			      struct seq_file *s, void *v,
> > +			      unsigned long flags)
> >  {
> > +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> 
> Change to:
> + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";

You couldn't come up with a more horrible hack, right?

Jiri was right with adding the flag to the base files, just the propagation
through the callbacks sucks. What's wrong with:

	struct rftype *rft = of->kn->priv;
	bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1614117

On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > >  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > -			      struct seq_file *s, void *v)
> > > +			      struct seq_file *s, void *v,
> > > +			      unsigned long flags)
> > >  {
> > > +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > 
> > Change to:
> > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> 
> You couldn't come up with a more horrible hack, right?
> 
> Jiri was right with adding the flag to the base files, just the propagation
> through the callbacks sucks. What's wrong with:
> 
> 	struct rftype *rft = of->kn->priv;
> 	bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;

You are absolutely correct.

Thanks.

-Fenghua

[toc] | [prev] | [next] | [standalone]

#1614128

On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > >  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > -			      struct seq_file *s, void *v)
> > > +			      struct seq_file *s, void *v,
> > > +			      unsigned long flags)
> > >  {
> > > +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > 
> > Change to:
> > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> 
> You couldn't come up with a more horrible hack, right?
> 
> Jiri was right with adding the flag to the base files, just the propagation
> through the callbacks sucks. What's wrong with:
> 
> 	struct rftype *rft = of->kn->priv;
> 	bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;

ok, I'll post the change

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1619514 — [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file

On Fri, Mar 31, 2017 at 05:55:12PM +0200, Jiri Olsa wrote:
> On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> > On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > >  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > > -			      struct seq_file *s, void *v)
> > > > +			      struct seq_file *s, void *v,
> > > > +			      unsigned long flags)
> > > >  {
> > > > +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > > 
> > > Change to:
> > > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> > 
> > You couldn't come up with a more horrible hack, right?
> > 
> > Jiri was right with adding the flag to the base files, just the propagation
> > through the callbacks sucks. What's wrong with:
> > 
> > 	struct rftype *rft = of->kn->priv;
> > 	bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
> 
> ok, I'll post the change

v2 attached

thanks,
jirka

---
While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.

Adding cpus_list file to provide cpumask list interface
to define group's cpus.

  # cd /sys/fs/resctrl/
  # echo 1-10 > krava/cpus_list
  # cat krava/cpus_list
  1-10
  # cat krava/cpus
  0007fe
  # cat cpus
  fffff9
  # cat cpus_list
  0,3-23

Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/include/asm/intel_rdt.h         |  4 ++++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..a1a6681738a5 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
 /* rdtgroup.flags */
 #define	RDT_DELETED		1
 
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST	1
+
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
 
@@ -56,6 +59,7 @@ struct rftype {
 	char			*name;
 	umode_t			mode;
 	struct kernfs_ops	*kf_ops;
+	unsigned long		flags;
 
 	int (*seq_show)(struct kernfs_open_file *of,
 			struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..b089d2452f1e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
 	.seq_show		= rdtgroup_seqfile_show,
 };
 
+static bool is_list(struct kernfs_open_file *of)
+{
+	struct rftype *rft = of->kn->priv;
+
+	return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
 static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 			      struct seq_file *s, void *v)
 {
@@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 
-	if (rdtgrp)
-		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
-	else
+	if (rdtgrp) {
+		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+			   cpumask_pr_args(&rdtgrp->cpu_mask));
+	} else
 		ret = -ENOENT;
 	rdtgroup_kn_unlock(of->kn);
 
@@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		goto unlock;
 	}
 
-	ret = cpumask_parse(buf, newmask);
+	if (is_list(of))
+		ret = cpulist_parse(buf, newmask);
+	else
+		ret = cpumask_parse(buf, newmask);
+
 	if (ret)
 		goto unlock;
 
@@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 		.seq_show	= rdtgroup_cpus_show,
 	},
 	{
+		.name		= "cpus_list",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.write		= rdtgroup_cpus_write,
+		.seq_show	= rdtgroup_cpus_show,
+		.flags		= RFTYPE_FLAGS_CPUS_LIST,
+	},
+	{
 		.name		= "tasks",
 		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1619524 — Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file

On Sun, Apr 09, 2017 at 06:49:29PM +0200, Jiri Olsa wrote:
> On Fri, Mar 31, 2017 at 05:55:12PM +0200, Jiri Olsa wrote:
> > On Fri, Mar 31, 2017 at 11:15:22AM +0200, Thomas Gleixner wrote:
> > > On Wed, 29 Mar 2017, Fenghua Yu wrote:
> > > > >  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
> > > > > -			      struct seq_file *s, void *v)
> > > > > +			      struct seq_file *s, void *v,
> > > > > +			      unsigned long flags)
> > > > >  {
> > > > > +	const char *fmt = flags & RFTYPE_FLAGS_CPUS_LIST ? "%*pbl\n" : "%*pb\n";
> > > > 
> > > > Change to:
> > > > + const char *fmt = strcmp(of->kn->priv, "cpus") ? "%*pbl\n" : "%*pb\n";
> > > 
> > > You couldn't come up with a more horrible hack, right?
> > > 
> > > Jiri was right with adding the flag to the base files, just the propagation
> > > through the callbacks sucks. What's wrong with:
> > > 
> > > 	struct rftype *rft = of->kn->priv;
> > > 	bool list = rtf->flags & RFTYPE_FLAGS_CPUS_LIST;
> > 
> > ok, I'll post the change
> 
> v2 attached
> 
> thanks,
> jirka
> 
> ---
> While playing with the resctrl interface I found it much
> easier to deal with cpumask list rather than just regular
> cpumask.
> 
> Adding cpus_list file to provide cpumask list interface
> to define group's cpus.
> 
>   # cd /sys/fs/resctrl/
>   # echo 1-10 > krava/cpus_list
>   # cat krava/cpus_list
>   1-10
>   # cat krava/cpus
>   0007fe
>   # cat cpus
>   fffff9
>   # cat cpus_list
>   0,3-23

Could you please have a right commit message and patch format?
git am can not pick up your commit message as you expected.

> 
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  arch/x86/include/asm/intel_rdt.h         |  4 ++++
>  arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 

Could you please add cpus_list info in intel_rdt_ui.txt?

> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
> index 3f313991c0b3..a1a6681738a5 100644
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -37,6 +37,9 @@ struct rdtgroup {
>  /* rdtgroup.flags */
>  #define	RDT_DELETED		1
>  
> +/* rftype.flags */
> +#define RFTYPE_FLAGS_CPUS_LIST	1
> +
>  /* List of all resource groups */
>  extern struct list_head rdt_all_groups;
>  
> @@ -56,6 +59,7 @@ struct rftype {
>  	char			*name;
>  	umode_t			mode;
>  	struct kernfs_ops	*kf_ops;
> +	unsigned long		flags;
>  
>  	int (*seq_show)(struct kernfs_open_file *of,
>  			struct seq_file *sf, void *v);
> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 9ac2a5cdd9c2..b089d2452f1e 100644
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
>  	.seq_show		= rdtgroup_seqfile_show,
>  };
>  
> +static bool is_list(struct kernfs_open_file *of)
> +{
> +	struct rftype *rft = of->kn->priv;
> +
> +	return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
> +}
> +
>  static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>  			      struct seq_file *s, void *v)
>  {
> @@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>  
>  	rdtgrp = rdtgroup_kn_lock_live(of->kn);
>  
> -	if (rdtgrp)
> -		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> -	else
> +	if (rdtgrp) {
> +		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> +			   cpumask_pr_args(&rdtgrp->cpu_mask));
> +	} else

Unbalanced braces around if-else. You can remove the { and } after if.

>  		ret = -ENOENT;
>  	rdtgroup_kn_unlock(of->kn);
>  
> @@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>  		goto unlock;
>  	}
>  
> -	ret = cpumask_parse(buf, newmask);
> +	if (is_list(of))
> +		ret = cpulist_parse(buf, newmask);
> +	else
> +		ret = cpumask_parse(buf, newmask);
> +
>  	if (ret)
>  		goto unlock;
>  
> @@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
>  		.seq_show	= rdtgroup_cpus_show,
>  	},
>  	{
> +		.name		= "cpus_list",
> +		.mode		= 0644,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.write		= rdtgroup_cpus_write,
> +		.seq_show	= rdtgroup_cpus_show,
> +		.flags		= RFTYPE_FLAGS_CPUS_LIST,
> +	},
> +	{
>  		.name		= "tasks",
>  		.mode		= 0644,
>  		.kf_ops		= &rdtgroup_kf_single_ops,
> -- 
> 1.8.3.1
> 

[toc] | [prev] | [next] | [standalone]

#1619688 — [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file

While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.

Adding cpus_list file to provide cpumask list interface
to define group's cpus.

  # cd /sys/fs/resctrl/
  # echo 1-10 > krava/cpus_list
  # cat krava/cpus_list
  1-10
  # cat krava/cpus
  0007fe
  # cat cpus
  fffff9
  # cat cpus_list
  0,3-23

Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/include/asm/intel_rdt.h         |  4 ++++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..a1a6681738a5 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
 /* rdtgroup.flags */
 #define	RDT_DELETED		1
 
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST	1
+
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
 
@@ -56,6 +59,7 @@ struct rftype {
 	char			*name;
 	umode_t			mode;
 	struct kernfs_ops	*kf_ops;
+	unsigned long		flags;
 
 	int (*seq_show)(struct kernfs_open_file *of,
 			struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..b089d2452f1e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
 	.seq_show		= rdtgroup_seqfile_show,
 };
 
+static bool is_list(struct kernfs_open_file *of)
+{
+	struct rftype *rft = of->kn->priv;
+
+	return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
 static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 			      struct seq_file *s, void *v)
 {
@@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 
-	if (rdtgrp)
-		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
-	else
+	if (rdtgrp) {
+		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+			   cpumask_pr_args(&rdtgrp->cpu_mask));
+	} else
 		ret = -ENOENT;
 	rdtgroup_kn_unlock(of->kn);
 
@@ -252,7 +260,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		goto unlock;
 	}
 
-	ret = cpumask_parse(buf, newmask);
+	if (is_list(of))
+		ret = cpulist_parse(buf, newmask);
+	else
+		ret = cpumask_parse(buf, newmask);
+
 	if (ret)
 		goto unlock;
 
@@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 		.seq_show	= rdtgroup_cpus_show,
 	},
 	{
+		.name		= "cpus_list",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.write		= rdtgroup_cpus_write,
+		.seq_show	= rdtgroup_cpus_show,
+		.flags		= RFTYPE_FLAGS_CPUS_LIST,
+	},
+	{
 		.name		= "tasks",
 		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1619761 — Re: [PATCHv3] x86/intel_rdt: Add cpus_list rdtgroup file

On Mon, 10 Apr 2017, Jiri Olsa wrote:
>  /* List of all resource groups */
>  extern struct list_head rdt_all_groups;
>  
> @@ -56,6 +59,7 @@ struct rftype {
>  	char			*name;
>  	umode_t			mode;
>  	struct kernfs_ops	*kf_ops;
> +	unsigned long		flags;

Lacks update in the kernel-doc comment.

> @@ -182,9 +189,10 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
>  
>  	rdtgrp = rdtgroup_kn_lock_live(of->kn);
>  
> -	if (rdtgrp)
> -		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> -	else
> +	if (rdtgrp) {
> +		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> +			   cpumask_pr_args(&rdtgrp->cpu_mask));
> +	} else
>  		ret = -ENOENT;

This lacks braces around the else path

> @@ -473,6 +485,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
>  		.seq_show	= rdtgroup_cpus_show,
>  	},
>  	{
> +		.name		= "cpus_list",
> +		.mode		= 0644,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.write		= rdtgroup_cpus_write,
> +		.seq_show	= rdtgroup_cpus_show,
> +		.flags		= RFTYPE_FLAGS_CPUS_LIST,
> +	},

Please add the new file to Documentation/x86/intel_rdt_ui.txt

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1619995 — [PATCHv4] x86/intel_rdt: Add cpus_list rdtgroup file

While playing with the resctrl interface I found it much
easier to deal with cpumask list rather than just regular
cpumask.

Adding cpus_list file to provide cpumask list interface
to define group's cpus.

  # cd /sys/fs/resctrl/
  # echo 1-10 > krava/cpus_list
  # cat krava/cpus_list
  1-10
  # cat krava/cpus
  0007fe
  # cat cpus
  fffff9
  # cat cpus_list
  0,3-23

Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Shaohua Li <shli@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 Documentation/x86/intel_rdt_ui.txt       |  3 +++
 arch/x86/include/asm/intel_rdt.h         |  5 +++++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 29 +++++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Documentation/x86/intel_rdt_ui.txt b/Documentation/x86/intel_rdt_ui.txt
index 3ea198460469..7029071e2373 100644
--- a/Documentation/x86/intel_rdt_ui.txt
+++ b/Documentation/x86/intel_rdt_ui.txt
@@ -59,6 +59,9 @@ There are three files associated with each group:
 	given to the default (root) group. You cannot remove CPUs
 	from the default group.
 
+"cpus_list": A bitmask *list* of logical CPUs assigned to this group.
+	Same rules apply like for the "cpus" file.
+
 "schemata": A list of all the resources available to this group.
 	Each resource has its own line and format - see below for
 	details.
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 3f313991c0b3..9069402f7497 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -37,6 +37,9 @@ struct rdtgroup {
 /* rdtgroup.flags */
 #define	RDT_DELETED		1
 
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST	1
+
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
 
@@ -49,6 +52,7 @@ struct rdtgroup {
  * @name: file name
  * @mode: access mode
  * @kf_ops: operations
+ * flags: RFTYPE_FLAGS_* flags
  * @seq_show: show content of the file
  * @write: write to the file
  */
@@ -56,6 +60,7 @@ struct rftype {
 	char			*name;
 	umode_t			mode;
 	struct kernfs_ops	*kf_ops;
+	unsigned long		flags;
 
 	int (*seq_show)(struct kernfs_open_file *of,
 			struct seq_file *sf, void *v);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9ac2a5cdd9c2..56b5bdaf98e1 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -174,6 +174,13 @@ static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
 	.seq_show		= rdtgroup_seqfile_show,
 };
 
+static bool is_list(struct kernfs_open_file *of)
+{
+	struct rftype *rft = of->kn->priv;
+
+	return rft->flags & RFTYPE_FLAGS_CPUS_LIST;
+}
+
 static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 			      struct seq_file *s, void *v)
 {
@@ -182,10 +189,12 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 
-	if (rdtgrp)
-		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
-	else
+	if (rdtgrp) {
+		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
+			   cpumask_pr_args(&rdtgrp->cpu_mask));
+	} else {
 		ret = -ENOENT;
+	}
 	rdtgroup_kn_unlock(of->kn);
 
 	return ret;
@@ -252,7 +261,11 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		goto unlock;
 	}
 
-	ret = cpumask_parse(buf, newmask);
+	if (is_list(of))
+		ret = cpulist_parse(buf, newmask);
+	else
+		ret = cpumask_parse(buf, newmask);
+
 	if (ret)
 		goto unlock;
 
@@ -473,6 +486,14 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 		.seq_show	= rdtgroup_cpus_show,
 	},
 	{
+		.name		= "cpus_list",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.write		= rdtgroup_cpus_write,
+		.seq_show	= rdtgroup_cpus_show,
+		.flags		= RFTYPE_FLAGS_CPUS_LIST,
+	},
+	{
 		.name		= "tasks",
 		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1619746 — Re: [PATCHv2] x86/intel_rdt: Add cpus_list rdtgroup file

On Sun, 9 Apr 2017, Fenghua Yu wrote:
> >  	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> >  
> > -	if (rdtgrp)
> > -		seq_printf(s, "%*pb\n", cpumask_pr_args(&rdtgrp->cpu_mask));
> > -	else
> > +	if (rdtgrp) {
> > +		seq_printf(s, is_list(of) ? "%*pbl\n" : "%*pb\n",
> > +			   cpumask_pr_args(&rdtgrp->cpu_mask));
> > +	} else
> 
> Unbalanced braces around if-else. You can remove the { and } after if.

Wrong. I've explained that a gazillion of times already.

http://lkml.kernel.org/r/alpine.DEB.2.20.1701171956290.3645@nanos

So yes, the else path wants braces.

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web