[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2671173.hSj4Ui9Upr@d46>
Date: Fri, 17 Oct 2014 09:55:19 +0200
From: Thomas Renninger <trenn@...e.de>
To: minyard@....org
Cc: linux-kernel@...r.kernel.org,
openipmi-developer@...ts.sourceforge.net
Subject: Re: [patch 2/3] ipmi: Remove ipmi_major module parameter and define global IPMI_MAJOR
Hi,
On Tuesday, October 14, 2014 08:26:26 PM Corey Minyard wrote:
> Sorry to top post on this, but you attached the file, so it's hard to reply
> inline.
>
> There's no way this can go in. There's not enough major device numbers
> for all the devices that exist, we have mechanisms to handle dynamically
> assigning numbers, and the IPMI driver just isn't important enough to
> warrant it's own major device number.
Hm, I always get 248, on 2 different machines. Just luck?
If 248 is still free (and according to major.h it is), why not?
> The particular mechanism to pass in the major number was just a temporary
> measure. It is no longer really necessary and could be removed.
Ok, if I read above right, you mean:
The module param to specify the major number can vanish.
But I must not try to invent a new MAJOR_IPMI definition in major.h.
Instead the whole thing can be dynamic (use MKDEV, remember the return
value and use it later to freeing again).
Thanks,
Thomas
> -corey
>
> On 10/14/2014 09:40 AM, trenn@...e.de wrote:
>
> There should be no need to specify the major number of /dev/ipmiX via module
> parameter. Others do not need this as well.
> This is the way msr.c (/dev/cpu/X/msr) is doing it as well.
>
> Side effect of this patch will be that the userspace interface cannot
> be disabled at kernel level anymore. But this can also be enforced by not
> letting userspace create the device file /dev/ipmiX.
>
> Signed-off-by: Thomas Renninger <trenn@...e.de>
> CC: minyard@....org
>
> Index: kernel_ipmi/drivers/char/ipmi/ipmi_devintf.c
> ===================================================================
> --- kernel_ipmi.orig/drivers/char/ipmi/ipmi_devintf.c
> +++ kernel_ipmi/drivers/char/ipmi/ipmi_devintf.c
> @@ -39,6 +39,7 @@
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> #include <linux/ipmi.h>
> +#include <linux/major.h>
> #include <linux/mutex.h>
> #include <linux/init.h>
> #include <linux/device.h>
> @@ -867,14 +868,6 @@ static const struct file_operations ipmi
>
> #define DEVICE_NAME "ipmidev"
>
> -static int ipmi_major;
> -module_param(ipmi_major, int, 0);
> -MODULE_PARM_DESC(ipmi_major, "Sets the major number of the IPMI device.
> By" - " default, or if you set it to zero, it will choose the next"
> - " available device. Setting it to -1 will disable the"
> - " interface. Other values will set the major device number"
> - " to that value.");
> -
> /* Keep track of the devices that are registered. */
> struct ipmi_reg_list {
> dev_t dev;
> @@ -887,7 +880,7 @@ static struct class *ipmi_class;
>
> static void ipmi_new_smi(int if_num, struct device *device)
> {
> - dev_t dev = MKDEV(ipmi_major, if_num);
> + dev_t dev = MKDEV(IPMI_MAJOR, if_num);
> struct ipmi_reg_list *entry;
>
> entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> @@ -906,7 +899,7 @@ static void ipmi_new_smi(int if_num, str
>
> static void ipmi_smi_gone(int if_num)
> {
> - dev_t dev = MKDEV(ipmi_major, if_num);
> + dev_t dev = MKDEV(IPMI_MAJOR, if_num);
> struct ipmi_reg_list *entry;
>
> mutex_lock(®_list_mutex);
> @@ -932,9 +925,6 @@ int __init init_ipmi_devintf(void)
> {
> int rv;
>
> - if (ipmi_major < 0)
> - return -EINVAL;
> -
> printk(KERN_INFO "ipmi device interface\n");
>
> ipmi_class = class_create(THIS_MODULE, "ipmi");
> @@ -943,20 +933,16 @@ int __init init_ipmi_devintf(void)
> return PTR_ERR(ipmi_class);
> }
>
> - rv = register_chrdev(ipmi_major, DEVICE_NAME, &ipmi_fops);
> + rv = register_chrdev(IPMI_MAJOR, DEVICE_NAME, &ipmi_fops);
> if (rv < 0) {
> class_destroy(ipmi_class);
> - printk(KERN_ERR "ipmi: can't get major %d\n", ipmi_major);
> + printk(KERN_ERR "ipmi: can't get major %d\n", IPMI_MAJOR);
> return rv;
> }
>
> - if (ipmi_major == 0) {
> - ipmi_major = rv;
> - }
> -
> rv = ipmi_smi_watcher_register(&smi_watcher);
> if (rv) {
> - unregister_chrdev(ipmi_major, DEVICE_NAME);
> + unregister_chrdev(IPMI_MAJOR, DEVICE_NAME);
> class_destroy(ipmi_class);
> printk(KERN_WARNING "ipmi: can't register smi watcher\n");
> return rv;
> @@ -977,5 +963,5 @@ void cleanup_ipmi_dev(void)
> mutex_unlock(®_list_mutex);
> class_destroy(ipmi_class);
> ipmi_smi_watcher_unregister(&smi_watcher);
> - unregister_chrdev(ipmi_major, DEVICE_NAME);
> + unregister_chrdev(IPMI_MAJOR, DEVICE_NAME);
> }
> Index: kernel_ipmi/include/uapi/linux/major.h
> ===================================================================
> --- kernel_ipmi.orig/include/uapi/linux/major.h
> +++ kernel_ipmi/include/uapi/linux/major.h
> @@ -173,6 +173,8 @@
>
> #define VIOTAPE_MAJOR 230
>
> +#define IPMI_MAJOR 248
> +
> #define BLOCK_EXT_MAJOR 259
> #define SCSI_OSD_MAJOR 260 /* open-osd's OSD scsi device
*/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists