Messages in this thread | | | From | Thomas Renninger <> | Subject | Re: [patch 2/3] ipmi: Remove ipmi_major module parameter and define global IPMI_MAJOR | Date | Fri, 17 Oct 2014 09:55:19 +0200 |
| |
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@suse.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@suse.de> > CC: minyard@acm.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 */
| |