* [PATCH] drivers: memory_open cleanup - lookup minor device number from devlist
@ 2009-04-20 4:05 Adriano dos Santos Fernandes
2009-04-21 0:01 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Adriano dos Santos Fernandes @ 2009-04-20 4:05 UTC (permalink / raw)
To: Ingo Molnar, Venkatesh Pallipadi, Suresh Siddha,
Arjan van de Ven, Andrew Morton, linux-kernel
memory_open ignores devlist and does a switch for each item, duplicating
code
and conditional definitions.
Clean it adding backing_dev_info to devlist and use it to lookup for the
minor
device.
Signed-off-by: Adriano dos Santos Fernandes <adrianosf@uol.com.br>
---
drivers/char/mem.c | 115
++++++++++++++++++++-------------------------------
1 files changed, 45 insertions(+), 70 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 8f05c38..9e350f2 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -861,59 +861,58 @@ static const struct file_operations kmsg_fops = {
.write = kmsg_write,
};
-static int memory_open(struct inode * inode, struct file * filp)
-{
- int ret = 0;
-
- lock_kernel();
- switch (iminor(inode)) {
- case 1:
- filp->f_op = &mem_fops;
- filp->f_mapping->backing_dev_info =
- &directly_mappable_cdev_bdi;
- break;
+static const struct {
+ unsigned int minor;
+ char *name;
+ umode_t mode;
+ const struct file_operations *fops;
+ struct backing_dev_info *dev_info;
+} devlist[] = { /* list of minor devices */
+ {1, "mem", S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops,
+ &directly_mappable_cdev_bdi},
#ifdef CONFIG_DEVKMEM
- case 2:
- filp->f_op = &kmem_fops;
- filp->f_mapping->backing_dev_info =
- &directly_mappable_cdev_bdi;
- break;
+ {2, "kmem", S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops,
+ &directly_mappable_cdev_bdi},
#endif
- case 3:
- filp->f_op = &null_fops;
- break;
+ {3, "null", S_IRUGO | S_IWUGO, &null_fops, NULL},
#ifdef CONFIG_DEVPORT
- case 4:
- filp->f_op = &port_fops;
- break;
+ {4, "port", S_IRUSR | S_IWUSR | S_IRGRP, &port_fops, NULL},
#endif
- case 5:
- filp->f_mapping->backing_dev_info = &zero_bdi;
- filp->f_op = &zero_fops;
- break;
- case 7:
- filp->f_op = &full_fops;
- break;
- case 8:
- filp->f_op = &random_fops;
- break;
- case 9:
- filp->f_op = &urandom_fops;
- break;
- case 11:
- filp->f_op = &kmsg_fops;
- break;
+ {5, "zero", S_IRUGO | S_IWUGO, &zero_fops, &zero_bdi},
+ {7, "full", S_IRUGO | S_IWUGO, &full_fops, NULL},
+ {8, "random", S_IRUGO | S_IWUSR, &random_fops, NULL},
+ {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops, NULL},
+ {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops, NULL},
#ifdef CONFIG_CRASH_DUMP
- case 12:
- filp->f_op = &oldmem_fops;
- break;
+ {12,"oldmem", S_IRUSR | S_IWUSR | S_IRGRP, &oldmem_fops, NULL},
#endif
- default:
- unlock_kernel();
- return -ENXIO;
+};
+
+static int memory_open(struct inode * inode, struct file * filp)
+{
+ int ret = 0;
+ int i;
+
+ lock_kernel();
+
+ for (i = 0; i < ARRAY_SIZE(devlist); i++) {
+ if (devlist[i].minor == iminor(inode)) {
+ filp->f_op = devlist[i].fops;
+ if (devlist[i].dev_info) {
+ filp->f_mapping->backing_dev_info =
+ devlist[i].dev_info;
+ }
+
+ break;
+ }
}
- if (filp->f_op && filp->f_op->open)
- ret = filp->f_op->open(inode,filp);
+
+ if (i == ARRAY_SIZE(devlist))
+ ret = -ENXIO;
+ else
+ if (filp->f_op && filp->f_op->open)
+ ret = filp->f_op->open(inode,filp);
+
unlock_kernel();
return ret;
}
@@ -922,30 +921,6 @@ static const struct file_operations memory_fops = {
.open = memory_open, /* just a selector for the real open */
};
-static const struct {
- unsigned int minor;
- char *name;
- umode_t mode;
- const struct file_operations *fops;
-} devlist[] = { /* list of minor devices */
- {1, "mem", S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops},
-#ifdef CONFIG_DEVKMEM
- {2, "kmem", S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops},
-#endif
- {3, "null", S_IRUGO | S_IWUGO, &null_fops},
-#ifdef CONFIG_DEVPORT
- {4, "port", S_IRUSR | S_IWUSR | S_IRGRP, &port_fops},
-#endif
- {5, "zero", S_IRUGO | S_IWUGO, &zero_fops},
- {7, "full", S_IRUGO | S_IWUGO, &full_fops},
- {8, "random", S_IRUGO | S_IWUSR, &random_fops},
- {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops},
- {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops},
-#ifdef CONFIG_CRASH_DUMP
- {12,"oldmem", S_IRUSR | S_IWUSR | S_IRGRP, &oldmem_fops},
-#endif
-};
-
static struct class *mem_class;
static int __init chr_dev_init(void)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers: memory_open cleanup - lookup minor device number from devlist
2009-04-20 4:05 [PATCH] drivers: memory_open cleanup - lookup minor device number from devlist Adriano dos Santos Fernandes
@ 2009-04-21 0:01 ` Andrew Morton
2009-04-21 0:27 ` Adriano dos Santos Fernandes
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2009-04-21 0:01 UTC (permalink / raw)
To: Adriano dos Santos Fernandes
Cc: mingo, venkatesh.pallipadi, suresh.b.siddha, arjan, linux-kernel
On Mon, 20 Apr 2009 01:05:23 -0300
Adriano dos Santos Fernandes <adrianosf@uol.com.br> wrote:
> memory_open ignores devlist and does a switch for each item, duplicating
> code
> and conditional definitions.
>
> Clean it adding backing_dev_info to devlist and use it to lookup for the
> minor
> device.
The patch looks reasonable, however your email client replaced all tabs
with spaces and I didn't really feel like fixing it all up.
I wonder what the lock_kernel() calls in memory_open() are doing.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers: memory_open cleanup - lookup minor device number from devlist
2009-04-21 0:01 ` Andrew Morton
@ 2009-04-21 0:27 ` Adriano dos Santos Fernandes
0 siblings, 0 replies; 3+ messages in thread
From: Adriano dos Santos Fernandes @ 2009-04-21 0:27 UTC (permalink / raw)
To: Andrew Morton
Cc: mingo, venkatesh.pallipadi, suresh.b.siddha, arjan, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 739 bytes --]
Andrew Morton wrote:
> On Mon, 20 Apr 2009 01:05:23 -0300
> Adriano dos Santos Fernandes <adrianosf@uol.com.br> wrote:
>
>
>> memory_open ignores devlist and does a switch for each item, duplicating
>> code
>> and conditional definitions.
>>
>> Clean it adding backing_dev_info to devlist and use it to lookup for the
>> minor
>> device.
>>
>
> The patch looks reasonable, however your email client replaced all tabs
> with spaces and I didn't really feel like fixing it all up.
>
I'm not having success to let Thunderbird make it right. I put the exact
same patch attached now.
> I wonder what the lock_kernel() calls in memory_open() are doing.
Hmm, not something I can say while submitting my first patch. ;-)
Adriano
[-- Attachment #2: memory_open.patch --]
[-- Type: text/x-patch, Size: 3846 bytes --]
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 8f05c38..9e350f2 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -861,59 +861,58 @@ static const struct file_operations kmsg_fops = {
.write = kmsg_write,
};
-static int memory_open(struct inode * inode, struct file * filp)
-{
- int ret = 0;
-
- lock_kernel();
- switch (iminor(inode)) {
- case 1:
- filp->f_op = &mem_fops;
- filp->f_mapping->backing_dev_info =
- &directly_mappable_cdev_bdi;
- break;
+static const struct {
+ unsigned int minor;
+ char *name;
+ umode_t mode;
+ const struct file_operations *fops;
+ struct backing_dev_info *dev_info;
+} devlist[] = { /* list of minor devices */
+ {1, "mem", S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops,
+ &directly_mappable_cdev_bdi},
#ifdef CONFIG_DEVKMEM
- case 2:
- filp->f_op = &kmem_fops;
- filp->f_mapping->backing_dev_info =
- &directly_mappable_cdev_bdi;
- break;
+ {2, "kmem", S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops,
+ &directly_mappable_cdev_bdi},
#endif
- case 3:
- filp->f_op = &null_fops;
- break;
+ {3, "null", S_IRUGO | S_IWUGO, &null_fops, NULL},
#ifdef CONFIG_DEVPORT
- case 4:
- filp->f_op = &port_fops;
- break;
+ {4, "port", S_IRUSR | S_IWUSR | S_IRGRP, &port_fops, NULL},
#endif
- case 5:
- filp->f_mapping->backing_dev_info = &zero_bdi;
- filp->f_op = &zero_fops;
- break;
- case 7:
- filp->f_op = &full_fops;
- break;
- case 8:
- filp->f_op = &random_fops;
- break;
- case 9:
- filp->f_op = &urandom_fops;
- break;
- case 11:
- filp->f_op = &kmsg_fops;
- break;
+ {5, "zero", S_IRUGO | S_IWUGO, &zero_fops, &zero_bdi},
+ {7, "full", S_IRUGO | S_IWUGO, &full_fops, NULL},
+ {8, "random", S_IRUGO | S_IWUSR, &random_fops, NULL},
+ {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops, NULL},
+ {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops, NULL},
#ifdef CONFIG_CRASH_DUMP
- case 12:
- filp->f_op = &oldmem_fops;
- break;
+ {12,"oldmem", S_IRUSR | S_IWUSR | S_IRGRP, &oldmem_fops, NULL},
#endif
- default:
- unlock_kernel();
- return -ENXIO;
+};
+
+static int memory_open(struct inode * inode, struct file * filp)
+{
+ int ret = 0;
+ int i;
+
+ lock_kernel();
+
+ for (i = 0; i < ARRAY_SIZE(devlist); i++) {
+ if (devlist[i].minor == iminor(inode)) {
+ filp->f_op = devlist[i].fops;
+ if (devlist[i].dev_info) {
+ filp->f_mapping->backing_dev_info =
+ devlist[i].dev_info;
+ }
+
+ break;
+ }
}
- if (filp->f_op && filp->f_op->open)
- ret = filp->f_op->open(inode,filp);
+
+ if (i == ARRAY_SIZE(devlist))
+ ret = -ENXIO;
+ else
+ if (filp->f_op && filp->f_op->open)
+ ret = filp->f_op->open(inode,filp);
+
unlock_kernel();
return ret;
}
@@ -922,30 +921,6 @@ static const struct file_operations memory_fops = {
.open = memory_open, /* just a selector for the real open */
};
-static const struct {
- unsigned int minor;
- char *name;
- umode_t mode;
- const struct file_operations *fops;
-} devlist[] = { /* list of minor devices */
- {1, "mem", S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops},
-#ifdef CONFIG_DEVKMEM
- {2, "kmem", S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops},
-#endif
- {3, "null", S_IRUGO | S_IWUGO, &null_fops},
-#ifdef CONFIG_DEVPORT
- {4, "port", S_IRUSR | S_IWUSR | S_IRGRP, &port_fops},
-#endif
- {5, "zero", S_IRUGO | S_IWUGO, &zero_fops},
- {7, "full", S_IRUGO | S_IWUGO, &full_fops},
- {8, "random", S_IRUGO | S_IWUSR, &random_fops},
- {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops},
- {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops},
-#ifdef CONFIG_CRASH_DUMP
- {12,"oldmem", S_IRUSR | S_IWUSR | S_IRGRP, &oldmem_fops},
-#endif
-};
-
static struct class *mem_class;
static int __init chr_dev_init(void)
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-04-21 0:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-20 4:05 [PATCH] drivers: memory_open cleanup - lookup minor device number from devlist Adriano dos Santos Fernandes
2009-04-21 0:01 ` Andrew Morton
2009-04-21 0:27 ` Adriano dos Santos Fernandes
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.