Re: [PATCH 16/39] ipmi: simplify procfs code
On Thu, Apr 19, 2018 at 10:29:29AM -0500, Corey Minyard wrote: > On 04/19/2018 07:41 AM, Christoph Hellwig wrote: >> Use remove_proc_subtree to remove the whole subtree on cleanup instead >> of a hand rolled list of proc entries, unwind the registration loop into >> individual calls. Switch to use proc_create_single to further simplify >> the code. > > I'm yanking all the proc code out of the IPMI driver in 3.18. So this is > probably > not necessary. Ok, I'll drop this patch.
Re: [PATCH 16/39] ipmi: simplify procfs code
On 04/19/2018 07:41 AM, Christoph Hellwig wrote:
Use remove_proc_subtree to remove the whole subtree on cleanup instead
of a hand rolled list of proc entries, unwind the registration loop into
individual calls. Switch to use proc_create_single to further simplify
the code.
I'm yanking all the proc code out of the IPMI driver in 3.18. So this
is probably
not necessary.
Thanks,
-corey
Signed-off-by: Christoph Hellwig
---
drivers/char/ipmi/ipmi_msghandler.c | 150 +---
drivers/char/ipmi/ipmi_si_intf.c| 47 +
drivers/char/ipmi/ipmi_ssif.c | 34 +--
include/linux/ipmi_smi.h| 8 +-
4 files changed, 33 insertions(+), 206 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c
b/drivers/char/ipmi/ipmi_msghandler.c
index 361148938801..c18db313e4c4 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -247,13 +247,6 @@ struct ipmi_my_addrinfo {
unsigned char lun;
};
-#ifdef CONFIG_IPMI_PROC_INTERFACE
-struct ipmi_proc_entry {
- char *name;
- struct ipmi_proc_entry *next;
-};
-#endif
-
/*
* Note that the product id, manufacturer id, guid, and device id are
* immutable in this structure, so dyn_mutex is not required for
@@ -430,10 +423,6 @@ struct ipmi_smi {
void *send_info;
#ifdef CONFIG_IPMI_PROC_INTERFACE
- /* A list of proc entries for this interface. */
- struct mutex proc_entry_lock;
- struct ipmi_proc_entry *proc_entries;
-
struct proc_dir_entry *proc_dir;
char proc_dir_name[10];
#endif
@@ -2358,18 +2347,6 @@ static int smi_ipmb_proc_show(struct seq_file *m, void
*v)
return 0;
}
-static int smi_ipmb_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_ipmb_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_ipmb_proc_ops = {
- .open = smi_ipmb_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
static int smi_version_proc_show(struct seq_file *m, void *v)
{
ipmi_smi_t intf = m->private;
@@ -2387,18 +2364,6 @@ static int smi_version_proc_show(struct seq_file *m,
void *v)
return 0;
}
-static int smi_version_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_version_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_version_proc_ops = {
- .open = smi_version_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
static int smi_stats_proc_show(struct seq_file *m, void *v)
{
ipmi_smi_t intf = m->private;
@@ -2462,95 +2427,45 @@ static int smi_stats_proc_show(struct seq_file *m, void
*v)
return 0;
}
-static int smi_stats_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_stats_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_stats_proc_ops = {
- .open = smi_stats_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name,
- const struct file_operations *proc_ops,
- void *data)
+ int (*show)(struct seq_file *, void *), void *data)
{
- intrv = 0;
- struct proc_dir_entry *file;
- struct ipmi_proc_entry *entry;
-
- /* Create a list element. */
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (!entry)
+ if (!proc_create_single_data(name, 0, smi->proc_dir, show, data))
return -ENOMEM;
- entry->name = kstrdup(name, GFP_KERNEL);
- if (!entry->name) {
- kfree(entry);
- return -ENOMEM;
- }
-
- file = proc_create_data(name, 0, smi->proc_dir, proc_ops, data);
- if (!file) {
- kfree(entry->name);
- kfree(entry);
- rv = -ENOMEM;
- } else {
- mutex_lock(&smi->proc_entry_lock);
- /* Stick it on the list. */
- entry->next = smi->proc_entries;
- smi->proc_entries = entry;
- mutex_unlock(&smi->proc_entry_lock);
- }
-
- return rv;
+ return 0;
}
EXPORT_SYMBOL(ipmi_smi_add_proc_entry);
static int add_proc_entries(ipmi_smi_t smi, int num)
{
- int rv = 0;
-
sprintf(smi->proc_dir_name, "%d", num);
smi->proc_dir = proc_mkdir(smi->proc_dir_name, proc_ipmi_root);
if (!smi->proc_dir)
- rv = -ENOMEM;
-
- if (rv == 0)
- rv = ipmi_smi_add_proc_entry(smi, "stats",
-
[PATCH 16/39] ipmi: simplify procfs code
Use remove_proc_subtree to remove the whole subtree on cleanup instead
of a hand rolled list of proc entries, unwind the registration loop into
individual calls. Switch to use proc_create_single to further simplify
the code.
Signed-off-by: Christoph Hellwig
---
drivers/char/ipmi/ipmi_msghandler.c | 150 +---
drivers/char/ipmi/ipmi_si_intf.c| 47 +
drivers/char/ipmi/ipmi_ssif.c | 34 +--
include/linux/ipmi_smi.h| 8 +-
4 files changed, 33 insertions(+), 206 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c
b/drivers/char/ipmi/ipmi_msghandler.c
index 361148938801..c18db313e4c4 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -247,13 +247,6 @@ struct ipmi_my_addrinfo {
unsigned char lun;
};
-#ifdef CONFIG_IPMI_PROC_INTERFACE
-struct ipmi_proc_entry {
- char *name;
- struct ipmi_proc_entry *next;
-};
-#endif
-
/*
* Note that the product id, manufacturer id, guid, and device id are
* immutable in this structure, so dyn_mutex is not required for
@@ -430,10 +423,6 @@ struct ipmi_smi {
void *send_info;
#ifdef CONFIG_IPMI_PROC_INTERFACE
- /* A list of proc entries for this interface. */
- struct mutex proc_entry_lock;
- struct ipmi_proc_entry *proc_entries;
-
struct proc_dir_entry *proc_dir;
char proc_dir_name[10];
#endif
@@ -2358,18 +2347,6 @@ static int smi_ipmb_proc_show(struct seq_file *m, void
*v)
return 0;
}
-static int smi_ipmb_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_ipmb_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_ipmb_proc_ops = {
- .open = smi_ipmb_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
static int smi_version_proc_show(struct seq_file *m, void *v)
{
ipmi_smi_t intf = m->private;
@@ -2387,18 +2364,6 @@ static int smi_version_proc_show(struct seq_file *m,
void *v)
return 0;
}
-static int smi_version_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_version_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_version_proc_ops = {
- .open = smi_version_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
static int smi_stats_proc_show(struct seq_file *m, void *v)
{
ipmi_smi_t intf = m->private;
@@ -2462,95 +2427,45 @@ static int smi_stats_proc_show(struct seq_file *m, void
*v)
return 0;
}
-static int smi_stats_proc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, smi_stats_proc_show, PDE_DATA(inode));
-}
-
-static const struct file_operations smi_stats_proc_ops = {
- .open = smi_stats_proc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
int ipmi_smi_add_proc_entry(ipmi_smi_t smi, char *name,
- const struct file_operations *proc_ops,
- void *data)
+ int (*show)(struct seq_file *, void *), void *data)
{
- intrv = 0;
- struct proc_dir_entry *file;
- struct ipmi_proc_entry *entry;
-
- /* Create a list element. */
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (!entry)
+ if (!proc_create_single_data(name, 0, smi->proc_dir, show, data))
return -ENOMEM;
- entry->name = kstrdup(name, GFP_KERNEL);
- if (!entry->name) {
- kfree(entry);
- return -ENOMEM;
- }
-
- file = proc_create_data(name, 0, smi->proc_dir, proc_ops, data);
- if (!file) {
- kfree(entry->name);
- kfree(entry);
- rv = -ENOMEM;
- } else {
- mutex_lock(&smi->proc_entry_lock);
- /* Stick it on the list. */
- entry->next = smi->proc_entries;
- smi->proc_entries = entry;
- mutex_unlock(&smi->proc_entry_lock);
- }
-
- return rv;
+ return 0;
}
EXPORT_SYMBOL(ipmi_smi_add_proc_entry);
static int add_proc_entries(ipmi_smi_t smi, int num)
{
- int rv = 0;
-
sprintf(smi->proc_dir_name, "%d", num);
smi->proc_dir = proc_mkdir(smi->proc_dir_name, proc_ipmi_root);
if (!smi->proc_dir)
- rv = -ENOMEM;
-
- if (rv == 0)
- rv = ipmi_smi_add_proc_entry(smi, "stats",
-&smi_stats_proc_ops,
-smi);
-
- if (rv == 0)
- rv = ipmi_smi_add_proc_entry(smi, "ipmb",
-
