diff mbox

[rfc,2/5] irq-am: add some debugfs exposure on tuning state

Message ID 20180205220316.30236-3-sagi@grimberg.me (mailing list archive)
State RFC
Headers show

Commit Message

Sagi Grimberg Feb. 5, 2018, 10:03 p.m. UTC 
Useful for local debugging

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 include/linux/irq-am.h |   2 +
 lib/irq-am.c           | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

Comments

kernel test robot Feb. 6, 2018, 4:04 p.m. UTC | #1 
Hi Sagi,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.15]
[also build test ERROR on next-20180206]
[cannot apply to linus/master rdma/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sagi-Grimberg/irq-am-Introduce-library-implementing-generic-adaptive-moderation/20180206-224501
config: i386-randconfig-s0-201805 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   lib/irq-am.c:17:8: error: type defaults to 'int' in declaration of 'DEFINE_IDA' [-Werror=implicit-int]
    static DEFINE_IDA(am_ida);
           ^~~~~~~~~~
   lib/irq-am.c:17:1: warning: parameter names (without types) in function declaration
    static DEFINE_IDA(am_ida);
    ^~~~~~
   lib/irq-am.c: In function 'irq_am_cleanup':
>> lib/irq-am.c:262:2: error: implicit declaration of function 'ida_simple_remove' [-Werror=implicit-function-declaration]
     ida_simple_remove(&am_ida, am->id);
     ^~~~~~~~~~~~~~~~~
   lib/irq-am.c:262:21: error: 'am_ida' undeclared (first use in this function)
     ida_simple_remove(&am_ida, am->id);
                        ^~~~~~
   lib/irq-am.c:262:21: note: each undeclared identifier is reported only once for each function it appears in
   lib/irq-am.c: In function 'irq_am_init':
>> lib/irq-am.c:276:11: error: implicit declaration of function 'ida_simple_get' [-Werror=implicit-function-declaration]
     am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
              ^~~~~~~~~~~~~~
   lib/irq-am.c:276:27: error: 'am_ida' undeclared (first use in this function)
     am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
                              ^~~~~~
   lib/irq-am.c: At top level:
   lib/irq-am.c:17:8: warning: 'DEFINE_IDA' declared 'static' but never defined [-Wunused-function]
    static DEFINE_IDA(am_ida);
           ^~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/ida_simple_remove +262 lib/irq-am.c

   257	
   258	void irq_am_cleanup(struct irq_am *am)
   259	{
   260		flush_work(&am->work);
   261		irq_am_deregister_debugfs(am);
 > 262		ida_simple_remove(&am_ida, am->id);
   263	}
   264	EXPORT_SYMBOL_GPL(irq_am_cleanup);
   265	
   266	void irq_am_init(struct irq_am *am, unsigned int nr_events,
   267		unsigned short nr_levels, unsigned short start_level, irq_am_fn *fn)
   268	{
   269		memset(am, 0, sizeof(*am));
   270		am->state = IRQ_AM_START_MEASURING;
   271		am->tune_state = IRQ_AM_GOING_UP;
   272		am->nr_levels = nr_levels;
   273		am->nr_events = nr_events;
   274		am->curr_level = start_level;
   275		am->program = fn;
 > 276		am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
   277		WARN_ON(am->id < 0);
   278		INIT_WORK(&am->work, irq_am_program_moderation_work);
   279		if (irq_am_register_debugfs(am))
   280			pr_warn("irq-am %d failed to register debugfs\n", am->id);
   281	}
   282	EXPORT_SYMBOL_GPL(irq_am_init);
   283	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Feb. 6, 2018, 5:38 p.m. UTC | #2 
Hi Sagi,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.15]
[also build test ERROR on next-20180206]
[cannot apply to linus/master rdma/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sagi-Grimberg/irq-am-Introduce-library-implementing-generic-adaptive-moderation/20180206-224501
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sh 

All error/warnings (new ones prefixed by >>):

>> lib/irq-am.c:17:8: error: type defaults to 'int' in declaration of 'DEFINE_IDA' [-Werror=implicit-int]
    static DEFINE_IDA(am_ida);
           ^~~~~~~~~~
>> lib/irq-am.c:17:1: warning: parameter names (without types) in function declaration
    static DEFINE_IDA(am_ida);
    ^~~~~~
   lib/irq-am.c: In function 'irq_am_cleanup':
>> lib/irq-am.c:262:2: error: implicit declaration of function 'ida_simple_remove'; did you mean 'simple_rename'? [-Werror=implicit-function-declaration]
     ida_simple_remove(&am_ida, am->id);
     ^~~~~~~~~~~~~~~~~
     simple_rename
>> lib/irq-am.c:262:21: error: 'am_ida' undeclared (first use in this function)
     ida_simple_remove(&am_ida, am->id);
                        ^~~~~~
   lib/irq-am.c:262:21: note: each undeclared identifier is reported only once for each function it appears in
   lib/irq-am.c: In function 'irq_am_init':
>> lib/irq-am.c:276:11: error: implicit declaration of function 'ida_simple_get'; did you mean 'simple_open'? [-Werror=implicit-function-declaration]
     am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
              ^~~~~~~~~~~~~~
              simple_open
   lib/irq-am.c:276:27: error: 'am_ida' undeclared (first use in this function)
     am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
                              ^~~~~~
   lib/irq-am.c: At top level:
   lib/irq-am.c:17:8: warning: 'DEFINE_IDA' declared 'static' but never defined [-Wunused-function]
    static DEFINE_IDA(am_ida);
           ^~~~~~~~~~
   cc1: some warnings being treated as errors

vim +17 lib/irq-am.c

    16	
  > 17	static DEFINE_IDA(am_ida);
    18	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Bart Van Assche Feb. 8, 2018, 1:24 a.m. UTC | #3 
On Tue, 2018-02-06 at 00:03 +0200, Sagi Grimberg wrote:
> +static int irq_am_register_debugfs(struct irq_am *am)

> +{

> +	char name[20];

> +

> +	snprintf(name, sizeof(name), "am%u", am->id);

> +	am->debugfs_dir = debugfs_create_dir(name,

> +				irq_am_debugfs_root);


How is a user expected to figure out which driver am%u is associated with?
Please make sure that these directories have a name that makes it easy for
users to figure out what driver the moderation settings apply to.

Thanks,

Bart.
Sagi Grimberg Feb. 12, 2018, 7:42 p.m. UTC | #4 
>> +static int irq_am_register_debugfs(struct irq_am *am)
>> +{
>> +	char name[20];
>> +
>> +	snprintf(name, sizeof(name), "am%u", am->id);
>> +	am->debugfs_dir = debugfs_create_dir(name,
>> +				irq_am_debugfs_root);
> 
> How is a user expected to figure out which driver am%u is associated with?
> Please make sure that these directories have a name that makes it easy for
> users to figure out what driver the moderation settings apply to.

You're right... It was just for me for local debugging at this point and
had every intention to get the debugfs into shape once I collect an
initial round of reviews.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/irq-am.h b/include/linux/irq-am.h
index 5ddd5ca268aa..18df315633a4 100644
--- a/include/linux/irq-am.h
+++ b/include/linux/irq-am.h
@@ -101,6 +101,8 @@  struct irq_am {
 
 	struct work_struct		work;
 	irq_am_fn			*program;
+	struct dentry			*debugfs_dir;
+	int				id;
 };
 
 void irq_am_add_event(struct irq_am *am);
diff --git a/lib/irq-am.c b/lib/irq-am.c
index ed7befd7a560..6cd2f131f5e8 100644
--- a/lib/irq-am.c
+++ b/lib/irq-am.c
@@ -12,6 +12,61 @@ 
  * more details.
  */
 #include <linux/irq-am.h>
+#include <linux/debugfs.h>
+
+static DEFINE_IDA(am_ida);
+
+#ifdef CONFIG_DEBUG_FS
+struct dentry *irq_am_debugfs_root;
+
+struct irq_am_debugfs_attr {
+	const char *name;
+	umode_t mode;
+	int (*show)(void *, struct seq_file *);
+	ssize_t (*write)(void *, const char __user *, size_t, loff_t *);
+};
+
+static int irq_am_tune_state_show(void *data, struct seq_file *m)
+{
+	struct irq_am *am = data;
+
+	seq_printf(m, "%d\n", am->tune_state);
+	return 0;
+}
+
+static int irq_am_curr_level_show(void *data, struct seq_file *m)
+{
+	struct irq_am *am = data;
+
+	seq_printf(m, "%d\n", am->curr_level);
+	return 0;
+}
+
+static int irq_am_debugfs_show(struct seq_file *m, void *v)
+{
+	const struct irq_am_debugfs_attr *attr = m->private;
+	void *data = d_inode(m->file->f_path.dentry->d_parent)->i_private;
+
+	return attr->show(data, m);
+}
+
+static int irq_am_debugfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, irq_am_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations irq_am_debugfs_fops = {
+	.open		= irq_am_debugfs_open,
+	.read		= seq_read,
+	.release	= seq_release,
+};
+
+static const struct irq_am_debugfs_attr irq_am_attrs[] = {
+	{"tune_state", 0400, irq_am_tune_state_show},
+	{"curr_level", 0400, irq_am_curr_level_show},
+	{},
+};
+#endif
 
 static void irq_am_try_step(struct irq_am *am)
 {
@@ -160,10 +215,51 @@  static void irq_am_program_moderation_work(struct work_struct *w)
 	am->state = IRQ_AM_START_MEASURING;
 }
 
+#ifdef CONFIG_DEBUG_FS
+static bool debugfs_create_files(struct dentry *parent, void *data,
+				 const struct irq_am_debugfs_attr *attr)
+{
+	d_inode(parent)->i_private = data;
+
+	for (; attr->name; attr++) {
+		if (!debugfs_create_file(attr->name, attr->mode, parent,
+					 (void *)attr, &irq_am_debugfs_fops))
+			return false;
+	}
+	return true;
+}
+
+static int irq_am_register_debugfs(struct irq_am *am)
+{
+	char name[20];
+
+	snprintf(name, sizeof(name), "am%u", am->id);
+	am->debugfs_dir = debugfs_create_dir(name,
+				irq_am_debugfs_root);
+	if (!am->debugfs_dir)
+		return -ENOMEM;
+
+	if (!debugfs_create_files(am->debugfs_dir, am,
+				irq_am_attrs))
+		return -ENOMEM;
+	return 0;
+}
+
+static void irq_am_deregister_debugfs(struct irq_am *am)
+{
+	debugfs_remove_recursive(am->debugfs_dir);
+}
+
+#else
+static int irq_am_register_debugfs(struct irq_am *am) {}
+static void irq_am_deregister_debugfs(struct irq_am *am) {}
+#endif
 
 void irq_am_cleanup(struct irq_am *am)
 {
 	flush_work(&am->work);
+	irq_am_deregister_debugfs(am);
+	ida_simple_remove(&am_ida, am->id);
 }
 EXPORT_SYMBOL_GPL(irq_am_cleanup);
 
@@ -177,6 +273,19 @@  void irq_am_init(struct irq_am *am, unsigned int nr_events,
 	am->nr_events = nr_events;
 	am->curr_level = start_level;
 	am->program = fn;
+	am->id = ida_simple_get(&am_ida, 0, 0, GFP_KERNEL);
+	WARN_ON(am->id < 0);
 	INIT_WORK(&am->work, irq_am_program_moderation_work);
+	if (irq_am_register_debugfs(am))
+		pr_warn("irq-am %d failed to register debugfs\n", am->id);
 }
 EXPORT_SYMBOL_GPL(irq_am_init);
+
+static __init int irq_am_setup(void)
+{
+#ifdef CONFIG_DEBUG_FS
+	irq_am_debugfs_root = debugfs_create_dir("irq_am", NULL);
+#endif
+	return 0;
+}
+subsys_initcall(irq_am_setup);