All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] x86/intel_rdt: Better diagnostics
@ 2017-09-25 23:39 Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 1/6] x86/intel_rdt: Add framework for better RDT UI diagnostics Luck, Tony
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

Chatting online with Boris to diagnose why his test cases for RDT
weren't working, we came up with either a good idea (in which case
I credit Boris) or a dumb one (in which case this is all my fault).

The basic problem is that there aren't many good error codes for
a file system interface to pass back to the user.  I'd resisted
adding printk() calls because it is a pain to parse the console
log, doubly so if you want to do it from a shell script that is
actually issuing the commands to RDT.

The answer is to add new file in the "info" directory that gives
the status of the last "command" to RDT (either a mkdir, or a
write to one of the control files).

I used the seq_buf* framework because I initially thought a single
command might result in multiple messages. But currently that isn't
true and we could potentially just use "strcpy()/sprintf()" to a
fixed buffer.  I didn't switch to that because the seq_buf* seems
very lightweight and allows for future extra messages while including
checking for exceeding the length of the buffer.

V1->V2: tglx: Add wrappers around functions that clear and append
	      to the seq_buf to assert that we hold the rdtgroup_mutex
	      [and drop the diagnostic from the place where we didn't]

Tony Luck (6):
  x86/intel_rdt: Add framework for better RDT UI diagnostics
  x86/intel_rdt: Add diagnostics when writing the schemata file
  x86/intel_rdt: Add diagnostics when writing the tasks file
  x86/intel_rdt: Add diagnostics when writing the cpus file
  x86/intel_rdt: Add diagnostics when making directories
  x86/intel_rdt: Add documentation for "info/last_cmd_status"

 Documentation/x86/intel_rdt_ui.txt          |  11 +++
 arch/x86/kernel/cpu/intel_rdt.h             |   7 ++
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c |  45 ++++++++---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c    | 112 +++++++++++++++++++++++++---
 4 files changed, 153 insertions(+), 22 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/6] x86/intel_rdt: Add framework for better RDT UI diagnostics
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 2/6] x86/intel_rdt: Add diagnostics when writing the schemata file Luck, Tony
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

Commands are given to the resctrl file system by making/removing
directories, or by writing to files.  When something goes wrong
the user is generally left wondering why they got:

	bash: echo: write error: Invalid argument

Add a new file "last_cmd_status" to the "info" directory that
will give the user some better clues on what went wrong.

Provide functions to clear and update last_cmd_status
that check that we hold the rdtgroup_mutex.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt.h          |  7 ++++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 56 ++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index ebaddaeef023..9d3148d88ec8 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -126,12 +126,15 @@ struct rdtgroup {
 #define RFTYPE_BASE			BIT(1)
 #define RF_CTRLSHIFT			4
 #define RF_MONSHIFT			5
+#define RF_TOPSHIFT			6
 #define RFTYPE_CTRL			BIT(RF_CTRLSHIFT)
 #define RFTYPE_MON			BIT(RF_MONSHIFT)
+#define RFTYPE_TOP			BIT(RF_TOPSHIFT)
 #define RFTYPE_RES_CACHE		BIT(8)
 #define RFTYPE_RES_MB			BIT(9)
 #define RF_CTRL_INFO			(RFTYPE_INFO | RFTYPE_CTRL)
 #define RF_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
+#define RF_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
 #define RF_CTRL_BASE			(RFTYPE_BASE | RFTYPE_CTRL)
 
 /* List of all resource groups */
@@ -408,6 +411,10 @@ union cpuid_0x10_x_edx {
 	unsigned int full;
 };
 
+void rdt_last_cmd_clear(void);
+void rdt_last_cmd_puts(const char *s);
+void rdt_last_cmd_printf(const char *fmt, ...);
+
 void rdt_ctrl_update(void *arg);
 struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn);
 void rdtgroup_kn_unlock(struct kernfs_node *kn);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index a869d4a073c5..9268122eb149 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -24,6 +24,7 @@
 #include <linux/fs.h>
 #include <linux/sysfs.h>
 #include <linux/kernfs.h>
+#include <linux/seq_buf.h>
 #include <linux/seq_file.h>
 #include <linux/sched/signal.h>
 #include <linux/sched/task.h>
@@ -51,6 +52,31 @@ static struct kernfs_node *kn_mongrp;
 /* Kernel fs node for "mon_data" directory under root */
 static struct kernfs_node *kn_mondata;
 
+struct seq_buf last_cmd_status;
+static char last_cmd_status_buf[512];
+
+void rdt_last_cmd_clear(void)
+{
+	lockdep_assert_held(&rdtgroup_mutex);
+	seq_buf_clear(&last_cmd_status);
+}
+
+void rdt_last_cmd_puts(const char *s)
+{
+	lockdep_assert_held(&rdtgroup_mutex);
+	seq_buf_puts(&last_cmd_status, s);
+}
+
+void rdt_last_cmd_printf(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	lockdep_assert_held(&rdtgroup_mutex);
+	seq_buf_vprintf(&last_cmd_status, fmt, ap);
+	va_end(ap);
+}
+
 /*
  * Trivial allocator for CLOSIDs. Since h/w only supports a small number,
  * we can keep a bitmap of free CLOSIDs in a single integer.
@@ -569,6 +595,21 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
+				    struct seq_file *seq, void *v)
+{
+	int len;
+
+	mutex_lock(&rdtgroup_mutex);
+	len = seq_buf_used(&last_cmd_status);
+	if (len)
+		seq_printf(seq, "%.*s", len, last_cmd_status_buf);
+	else
+		seq_puts(seq, "ok\n");
+	mutex_unlock(&rdtgroup_mutex);
+	return 0;
+}
+
 static int rdt_num_closids_show(struct kernfs_open_file *of,
 				struct seq_file *seq, void *v)
 {
@@ -686,6 +727,13 @@ static ssize_t max_threshold_occ_write(struct kernfs_open_file *of,
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
+		.name		= "last_cmd_status",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdt_last_cmd_status_show,
+		.fflags		= RF_TOP_INFO,
+	},
+	{
 		.name		= "num_closids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
@@ -855,6 +903,10 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 		return PTR_ERR(kn_info);
 	kernfs_get(kn_info);
 
+	ret = rdtgroup_add_files(kn_info, RF_TOP_INFO);
+	if (ret)
+		goto out_destroy;
+
 	for_each_alloc_enabled_rdt_resource(r) {
 		fflags =  r->fflags | RF_CTRL_INFO;
 		ret = rdtgroup_mkdir_info_resdir(r, r->name, fflags);
@@ -1156,6 +1208,7 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 out_cdp:
 	cdp_disable();
 out:
+	seq_buf_clear(&last_cmd_status);
 	mutex_unlock(&rdtgroup_mutex);
 
 	return dentry;
@@ -1902,6 +1955,9 @@ int __init rdtgroup_init(void)
 {
 	int ret = 0;
 
+	seq_buf_init(&last_cmd_status, last_cmd_status_buf,
+		     sizeof(last_cmd_status_buf));
+
 	ret = rdtgroup_setup_root();
 	if (ret)
 		return ret;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/6] x86/intel_rdt: Add diagnostics when writing the schemata file
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 1/6] x86/intel_rdt: Add framework for better RDT UI diagnostics Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 3/6] x86/intel_rdt: Add diagnostics when writing the tasks file Luck, Tony
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

Save helpful descriptions of what went wrong when writing a
schemata file.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c | 49 ++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
index f6ea94f8954a..f29b4c21e7d4 100644
--- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
@@ -42,15 +42,22 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 	/*
 	 * Only linear delay values is supported for current Intel SKUs.
 	 */
-	if (!r->membw.delay_linear)
+	if (!r->membw.delay_linear) {
+		rdt_last_cmd_puts("No support for non-linear MB domains\n");
 		return false;
+	}
 
 	ret = kstrtoul(buf, 10, &bw);
-	if (ret)
+	if (ret) {
+		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
 		return false;
+	}
 
-	if (bw < r->membw.min_bw || bw > r->default_ctrl)
+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
+		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
+				    r->membw.min_bw, r->default_ctrl);
 		return false;
+	}
 
 	*data = roundup(bw, (unsigned long)r->membw.bw_gran);
 	return true;
@@ -60,8 +67,10 @@ int parse_bw(char *buf, struct rdt_resource *r, struct rdt_domain *d)
 {
 	unsigned long data;
 
-	if (d->have_new_ctrl)
+	if (d->have_new_ctrl) {
+		rdt_last_cmd_printf("duplicate domain %d\n", d->id);
 		return -EINVAL;
+	}
 
 	if (!bw_validate(buf, &data, r))
 		return -EINVAL;
@@ -84,20 +93,29 @@ static bool cbm_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 	int ret;
 
 	ret = kstrtoul(buf, 16, &val);
-	if (ret)
+	if (ret) {
+		rdt_last_cmd_printf("non-hex character in mask %s\n", buf);
 		return false;
+	}
 
-	if (val == 0 || val > r->default_ctrl)
+	if (val == 0 || val > r->default_ctrl) {
+		rdt_last_cmd_puts("mask out of range\n");
 		return false;
+	}
 
 	first_bit = find_first_bit(&val, cbm_len);
 	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
 
-	if (find_next_bit(&val, cbm_len, zero_bit) < cbm_len)
+	if (find_next_bit(&val, cbm_len, zero_bit) < cbm_len) {
+		rdt_last_cmd_printf("mask %lx has non-consecutive 1-bits\n", val);
 		return false;
+	}
 
-	if ((zero_bit - first_bit) < r->cache.min_cbm_bits)
+	if ((zero_bit - first_bit) < r->cache.min_cbm_bits) {
+		rdt_last_cmd_printf("Need at least %d bits in mask\n",
+				    r->cache.min_cbm_bits);
 		return false;
+	}
 
 	*data = val;
 	return true;
@@ -111,8 +129,10 @@ int parse_cbm(char *buf, struct rdt_resource *r, struct rdt_domain *d)
 {
 	unsigned long data;
 
-	if (d->have_new_ctrl)
+	if (d->have_new_ctrl) {
+		rdt_last_cmd_printf("duplicate domain %d\n", d->id);
 		return -EINVAL;
+	}
 
 	if(!cbm_validate(buf, &data, r))
 		return -EINVAL;
@@ -139,8 +159,10 @@ static int parse_line(char *line, struct rdt_resource *r)
 		return 0;
 	dom = strsep(&line, ";");
 	id = strsep(&dom, "=");
-	if (!dom || kstrtoul(id, 10, &dom_id))
+	if (!dom || kstrtoul(id, 10, &dom_id)) {
+		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
 		return -EINVAL;
+	}
 	dom = strim(dom);
 	list_for_each_entry(d, &r->domains, list) {
 		if (d->id == dom_id) {
@@ -196,6 +218,7 @@ static int rdtgroup_parse_resource(char *resname, char *tok, int closid)
 		if (!strcmp(resname, r->name) && closid < r->num_closid)
 			return parse_line(tok, r);
 	}
+	rdt_last_cmd_printf("unknown/unsupported resource name '%s'\n", resname);
 	return -EINVAL;
 }
 
@@ -209,8 +232,10 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 	int closid, ret = 0;
 
 	/* Valid input requires a trailing newline */
-	if (nbytes == 0 || buf[nbytes - 1] != '\n')
+	if (nbytes == 0 || buf[nbytes - 1] != '\n') {
+		seq_buf_puts(&last_cmd_status, "no trailing newline\n");
 		return -EINVAL;
+	}
 	buf[nbytes - 1] = '\0';
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
@@ -218,6 +243,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 		rdtgroup_kn_unlock(of->kn);
 		return -ENOENT;
 	}
+	rdt_last_cmd_clear();
 
 	closid = rdtgrp->closid;
 
@@ -229,6 +255,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 	while ((tok = strsep(&buf, "\n")) != NULL) {
 		resname = strim(strsep(&tok, ":"));
 		if (!tok) {
+			rdt_last_cmd_puts("Missing ':'\n");
 			ret = -EINVAL;
 			goto out;
 		}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/6] x86/intel_rdt: Add diagnostics when writing the tasks file
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 1/6] x86/intel_rdt: Add framework for better RDT UI diagnostics Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 2/6] x86/intel_rdt: Add diagnostics when writing the schemata file Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 4/6] x86/intel_rdt: Add diagnostics when writing the cpus file Luck, Tony
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

About the only tricky case is trying to move a task into a monitor
group that is a subdirectory of a different control group. But cover
the simple cases too.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c |  4 +---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c    | 15 +++++++++++----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
index f29b4c21e7d4..30aeb267cbd2 100644
--- a/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_ctrlmondata.c
@@ -232,10 +232,8 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 	int closid, ret = 0;
 
 	/* Valid input requires a trailing newline */
-	if (nbytes == 0 || buf[nbytes - 1] != '\n') {
-		seq_buf_puts(&last_cmd_status, "no trailing newline\n");
+	if (nbytes == 0 || buf[nbytes - 1] != '\n')
 		return -EINVAL;
-	}
 	buf[nbytes - 1] = '\0';
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 9268122eb149..df6373680a59 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -478,6 +478,7 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
 		 */
 		atomic_dec(&rdtgrp->waitcount);
 		kfree(callback);
+		rdt_last_cmd_puts("task exited\n");
 	} else {
 		/*
 		 * For ctrl_mon groups move both closid and rmid.
@@ -488,10 +489,12 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
 			tsk->closid = rdtgrp->closid;
 			tsk->rmid = rdtgrp->mon.rmid;
 		} else if (rdtgrp->type == RDTMON_GROUP) {
-			if (rdtgrp->mon.parent->closid == tsk->closid)
+			if (rdtgrp->mon.parent->closid == tsk->closid) {
 				tsk->rmid = rdtgrp->mon.rmid;
-			else
+			} else {
+				rdt_last_cmd_puts("Can't move task to different control group\n");
 				ret = -EINVAL;
+			}
 		}
 	}
 	return ret;
@@ -510,8 +513,10 @@ static int rdtgroup_task_write_permission(struct task_struct *task,
 	 */
 	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
 	    !uid_eq(cred->euid, tcred->uid) &&
-	    !uid_eq(cred->euid, tcred->suid))
+	    !uid_eq(cred->euid, tcred->suid)) {
+		rdt_last_cmd_printf("No permission to move task %d\n", task->pid);
 		ret = -EPERM;
+	}
 
 	put_cred(tcred);
 	return ret;
@@ -528,6 +533,7 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
 		tsk = find_task_by_vpid(pid);
 		if (!tsk) {
 			rcu_read_unlock();
+			rdt_last_cmd_printf("No task %d\n", pid);
 			return -ESRCH;
 		}
 	} else {
@@ -555,6 +561,7 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
 	if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
 		return -EINVAL;
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+	rdt_last_cmd_clear();
 
 	if (rdtgrp)
 		ret = rdtgroup_move_task(pid, rdtgrp, of);
@@ -1208,7 +1215,7 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 out_cdp:
 	cdp_disable();
 out:
-	seq_buf_clear(&last_cmd_status);
+	rdt_last_cmd_clear();
 	mutex_unlock(&rdtgroup_mutex);
 
 	return dentry;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/6] x86/intel_rdt: Add diagnostics when writing the cpus file
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
                   ` (2 preceding siblings ...)
  2017-09-25 23:39 ` [PATCH v2 3/6] x86/intel_rdt: Add diagnostics when writing the tasks file Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 5/6] x86/intel_rdt: Add diagnostics when making directories Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 6/6] x86/intel_rdt: Add documentation for "info/last_cmd_status" Luck, Tony
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

Can't add a cpu to a monitor group unless it belongs to parent
group. Can't delete cpus from the default group.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index df6373680a59..1674f7871b06 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -264,8 +264,10 @@ static int cpus_mon_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
 
 	/* Check whether cpus belong to parent ctrl group */
 	cpumask_andnot(tmpmask, newmask, &prgrp->cpu_mask);
-	if (cpumask_weight(tmpmask))
+	if (cpumask_weight(tmpmask)) {
+		rdt_last_cmd_puts("can only add CPUs to mongroup that belong to parent\n");
 		return -EINVAL;
+	}
 
 	/* Check whether cpus are dropped from this group */
 	cpumask_andnot(tmpmask, &rdtgrp->cpu_mask, newmask);
@@ -317,8 +319,10 @@ static int cpus_ctrl_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
 	cpumask_andnot(tmpmask, &rdtgrp->cpu_mask, newmask);
 	if (cpumask_weight(tmpmask)) {
 		/* Can't drop from default group */
-		if (rdtgrp == &rdtgroup_default)
+		if (rdtgrp == &rdtgroup_default) {
+			rdt_last_cmd_puts("Can't drop CPUs from default group\n");
 			return -EINVAL;
+		}
 
 		/* Give any dropped cpus to rdtgroup_default */
 		cpumask_or(&rdtgroup_default.cpu_mask,
@@ -383,8 +387,10 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 	}
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+	rdt_last_cmd_clear();
 	if (!rdtgrp) {
 		ret = -ENOENT;
+		rdt_last_cmd_puts("directory was removed\n");
 		goto unlock;
 	}
 
@@ -393,13 +399,16 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 	else
 		ret = cpumask_parse(buf, newmask);
 
-	if (ret)
+	if (ret) {
+		rdt_last_cmd_puts("bad cpu list/mask\n");
 		goto unlock;
+	}
 
 	/* check that user didn't specify any offline cpus */
 	cpumask_andnot(tmpmask, newmask, cpu_online_mask);
 	if (cpumask_weight(tmpmask)) {
 		ret = -EINVAL;
+		rdt_last_cmd_puts("can only assign online cpus\n");
 		goto unlock;
 	}
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 5/6] x86/intel_rdt: Add diagnostics when making directories
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
                   ` (3 preceding siblings ...)
  2017-09-25 23:39 ` [PATCH v2 4/6] x86/intel_rdt: Add diagnostics when writing the cpus file Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  2017-09-25 23:39 ` [PATCH v2 6/6] x86/intel_rdt: Add documentation for "info/last_cmd_status" Luck, Tony
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

Mostly this is about running out of RMIDs or CLOSIDs. Other
errors are various internal errors.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 1674f7871b06..c015cc1c2e8e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -1593,8 +1593,10 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 	int ret;
 
 	prdtgrp = rdtgroup_kn_lock_live(prgrp_kn);
+	rdt_last_cmd_clear();
 	if (!prdtgrp) {
 		ret = -ENODEV;
+		rdt_last_cmd_puts("directory was removed\n");
 		goto out_unlock;
 	}
 
@@ -1602,6 +1604,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 	rdtgrp = kzalloc(sizeof(*rdtgrp), GFP_KERNEL);
 	if (!rdtgrp) {
 		ret = -ENOSPC;
+		rdt_last_cmd_puts("kernel out of memory\n");
 		goto out_unlock;
 	}
 	*r = rdtgrp;
@@ -1613,6 +1616,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 	kn = kernfs_create_dir(parent_kn, name, mode, rdtgrp);
 	if (IS_ERR(kn)) {
 		ret = PTR_ERR(kn);
+		rdt_last_cmd_puts("kernfs create error\n");
 		goto out_free_rgrp;
 	}
 	rdtgrp->kn = kn;
@@ -1626,24 +1630,32 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 	kernfs_get(kn);
 
 	ret = rdtgroup_kn_set_ugid(kn);
-	if (ret)
+	if (ret) {
+		rdt_last_cmd_puts("kernfs perm error\n");
 		goto out_destroy;
+	}
 
 	files = RFTYPE_BASE | RFTYPE_CTRL;
 	files = RFTYPE_BASE | BIT(RF_CTRLSHIFT + rtype);
 	ret = rdtgroup_add_files(kn, files);
-	if (ret)
+	if (ret) {
+		rdt_last_cmd_puts("kernfs fill error\n");
 		goto out_destroy;
+	}
 
 	if (rdt_mon_capable) {
 		ret = alloc_rmid();
-		if (ret < 0)
+		if (ret < 0) {
+			rdt_last_cmd_puts("out of RMIDs\n");
 			goto out_destroy;
+		}
 		rdtgrp->mon.rmid = ret;
 
 		ret = mkdir_mondata_all(kn, rdtgrp, &rdtgrp->mon.mon_data_kn);
-		if (ret)
+		if (ret) {
+			rdt_last_cmd_puts("kernfs subdir error\n");
 			goto out_idfree;
+		}
 	}
 	kernfs_activate(kn);
 
@@ -1721,8 +1733,10 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
 
 	kn = rdtgrp->kn;
 	ret = closid_alloc();
-	if (ret < 0)
+	if (ret < 0) {
+		rdt_last_cmd_puts("out of CLOSIDs\n");
 		goto out_common_fail;
+	}
 	closid = ret;
 
 	rdtgrp->closid = closid;
@@ -1734,8 +1748,10 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
 		 * of tasks and cpus to monitor.
 		 */
 		ret = mongroup_create_dir(kn, NULL, "mon_groups", NULL);
-		if (ret)
+		if (ret) {
+			rdt_last_cmd_puts("kernfs subdir error\n");
 			goto out_id_free;
+		}
 	}
 
 	goto out_unlock;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 6/6] x86/intel_rdt: Add documentation for "info/last_cmd_status"
  2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
                   ` (4 preceding siblings ...)
  2017-09-25 23:39 ` [PATCH v2 5/6] x86/intel_rdt: Add diagnostics when making directories Luck, Tony
@ 2017-09-25 23:39 ` Luck, Tony
  5 siblings, 0 replies; 7+ messages in thread
From: Luck, Tony @ 2017-09-25 23:39 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Tony Luck, linux-kernel, Boris Petkov, Fenghua Yu,
	Reinette Chatre, Steven Rostedt, Vikas Shivappa

From: Tony Luck <tony.luck@intel.com>

New file in the "info" directory helps diagnose what went wrong
when using the /sys/fs/resctrl file system

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 Documentation/x86/intel_rdt_ui.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/x86/intel_rdt_ui.txt b/Documentation/x86/intel_rdt_ui.txt
index 4d8848e4e224..6851854cf69d 100644
--- a/Documentation/x86/intel_rdt_ui.txt
+++ b/Documentation/x86/intel_rdt_ui.txt
@@ -87,6 +87,17 @@ with the following files:
 			bytes) at which a previously used LLC_occupancy
 			counter can be considered for re-use.
 
+Finally, in the top level of the "info" directory there is a file
+named "last_cmd_status". This is reset with every "command" issued
+via the file system (making new directories or writing to any of the
+control files). If the command was successful, it will read as "ok".
+If the command failed, it will provide more information that can be
+conveyed in the error returns from file operations. E.g.
+
+	# echo L3:0=f7 > schemata
+	bash: echo: write error: Invalid argument
+	# cat info/last_cmd_status
+	mask f7 has non-consecutive 1-bits
 
 Resource alloc and monitor groups
 ---------------------------------
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-09-25 23:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-25 23:39 [PATCH v2 0/6] x86/intel_rdt: Better diagnostics Luck, Tony
2017-09-25 23:39 ` [PATCH v2 1/6] x86/intel_rdt: Add framework for better RDT UI diagnostics Luck, Tony
2017-09-25 23:39 ` [PATCH v2 2/6] x86/intel_rdt: Add diagnostics when writing the schemata file Luck, Tony
2017-09-25 23:39 ` [PATCH v2 3/6] x86/intel_rdt: Add diagnostics when writing the tasks file Luck, Tony
2017-09-25 23:39 ` [PATCH v2 4/6] x86/intel_rdt: Add diagnostics when writing the cpus file Luck, Tony
2017-09-25 23:39 ` [PATCH v2 5/6] x86/intel_rdt: Add diagnostics when making directories Luck, Tony
2017-09-25 23:39 ` [PATCH v2 6/6] x86/intel_rdt: Add documentation for "info/last_cmd_status" Luck, Tony

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.