Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[linux-dvb] [PATCH] Lock adapter modules when devices are open



The patch attached (for dvb-kernel only) causes the module use count to be automatically incremented when any of the frontend, demux, etc devices are openned.

When the device is closed the count decremented so the module can be safely removed. I have only done a patch for the dvb-kernel driver.

This prevents oopses which occur if one of the adpater modules is removed when you accidentally have the device open (e.g. femon or dvbstream running).

The dvb-core code does the actual locking, but it is a bit un-obvious as the open/release paths are quite complicated with all the different adapters and devices. It looks ok to me, but I wouldn't want to guarantee that it is completely race-free.

I've tested it with my budget and budget-ci cards and it appears to all work OK. It nees a one line change in each of the adapters to make the locking work (passing a refernce to the module to be locked). I have included these changes in the patch and it all compiles OK for me, but is untested.

Hopefully this addresses one of your TODO list items!

Jon

diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/b2c2/skystar2.c dvb-test/linux/drivers/media/dvb/b2c2/skystar2.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/b2c2/skystar2.c	2003-09-11 17:35:55.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/b2c2/skystar2.c	2003-09-16 20:51:24.000000000 +0100
@@ -2384,7 +2384,7 @@
 	if (DriverInitialize(pdev) != 0)
 		return -ENODEV;
 
-	dvb_register_adapter(&dvb_adapter, skystar2_pci_driver.name);
+	dvb_register_adapter(&dvb_adapter, skystar2_pci_driver.name, THIS_MODULE);
 
 	if (dvb_adapter == NULL) {
 		printk("%s: Error registering DVB adapter\n", __FUNCTION__);
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/bt8xx/dvb-bt8xx.c dvb-test/linux/drivers/media/dvb/bt8xx/dvb-bt8xx.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/bt8xx/dvb-bt8xx.c	2003-09-11 17:35:55.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/bt8xx/dvb-bt8xx.c	2003-09-16 22:30:37.000000000 +0100
@@ -152,7 +152,8 @@
 	
 	}
 
-	if ((result = dvb_register_adapter(&card->adapter, card_name)) < 0) {
+	if ((result = dvb_register_adapter(&card->adapter, 
+					   card_name, THIS_MODULE)) < 0) {
 	
 		printk("dvb_bt8xx: dvb_register_adapter failed (errno = %d)\n", result);
 		
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dmxdev.c dvb-test/linux/drivers/media/dvb/dvb-core/dmxdev.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dmxdev.c	2003-06-18 19:44:20.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/dvb-core/dmxdev.c	2003-09-17 22:57:17.000000000 +0100
@@ -250,6 +250,7 @@
 		}
 	}
 	up(&dmxdev->mutex);
+	dvb_device_release(dvbdev);
 	return 0;
 }
 
@@ -992,8 +993,14 @@
 {
 	struct dmxdev_filter *dmxdevfilter = dvb_dmxdev_file_to_filter(file);
 	struct dmxdev *dmxdev = dmxdevfilter->dev;
+	struct dvb_device *dvbdev= dmxdev->dvbdev;
+	int ret;
 
-	return dvb_dmxdev_filter_free(dmxdev, dmxdevfilter);
+	ret = dvb_dmxdev_filter_free(dmxdev, dmxdevfilter);
+	if (!ret) {
+	        dvb_device_release(dvbdev);
+	}
+	return ret;
 }
 
 
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dvbdev.c dvb-test/linux/drivers/media/dvb/dvb-core/dvbdev.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dvbdev.c	2003-09-11 17:35:55.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/dvb-core/dvbdev.c	2003-09-17 23:10:38.000000000 +0100
@@ -79,26 +79,38 @@
 static int dvb_device_open(struct inode *inode, struct file *file)
 {
 	struct dvb_device *dvbdev;
+	struct file_operations *old_fops;
+	int err = -EOPNOTSUPP;
 	
 	dvbdev = dvbdev_find_device (iminor(inode));
 
-	if (dvbdev && dvbdev->fops) {
-		int err = 0;
-		struct file_operations *old_fops;
-
-		file->private_data = dvbdev;
-		old_fops = file->f_op;
-                file->f_op = fops_get(dvbdev->fops);
-                if(file->f_op->open)
-                        err = file->f_op->open(inode,file);
-                if (err) {
-                        fops_put(file->f_op);
-                        file->f_op = fops_get(old_fops);
-                }
-                fops_put(old_fops);
-                return err;
+	if (!dvbdev || !dvbdev->fops || !dvbdev->adapter 
+	    || !try_module_get(dvbdev->adapter->module)) {
+	        return -ENODEV;
 	}
-	return -ENODEV;
+
+	file->private_data = dvbdev;
+	old_fops = file->f_op;
+	file->f_op = fops_get(dvbdev->fops);
+	if(file->f_op->open)
+	       err = file->f_op->open(inode,file);
+	if (err) {
+	        fops_put(file->f_op);
+	        file->f_op = fops_get(old_fops);
+	        module_put(dvbdev->adapter->module);
+	}
+	fops_put(old_fops);
+	return err;
+}
+
+/* Needs to be to drop module count when not 
+ * using dvb_generic_release.
+ */
+void dvb_device_release(struct dvb_device *dvbdev)
+{
+        if (dvbdev && dvbdev->adapter) {
+	        module_put(dvbdev->adapter->module);        
+        }
 }
 
 
@@ -150,6 +162,7 @@
 	}
 	
 	dvbdev->users++;
+	dvb_device_release(dvbdev);
 	return 0;
 }
 
@@ -287,7 +300,8 @@
 }
 
 
-int dvb_register_adapter(struct dvb_adapter **padap, const char *name)
+int dvb_register_adapter(struct dvb_adapter **padap, const char *name,
+			 struct module *module)
 {
 	struct dvb_adapter *adap;
 	int num;
@@ -321,6 +335,7 @@
 #endif
 	adap->num = num;
 	adap->name = name;
+	adap->module = module;
 
 	list_add_tail (&adap->list_head, &dvb_adapter_list);
 
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dvbdev.h dvb-test/linux/drivers/media/dvb/dvb-core/dvbdev.h
--- dvb-kernel-cvs/linux/drivers/media/dvb/dvb-core/dvbdev.h	2003-08-25 22:42:23.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/dvb-core/dvbdev.h	2003-09-17 21:59:16.000000000 +0100
@@ -52,6 +52,7 @@
 	struct list_head device_list;
 	const char *name;
 	u8 proposed_mac [6];
+	struct module *module;
 };
 
 
@@ -79,7 +80,10 @@
 };
 
 
-extern int dvb_register_adapter (struct dvb_adapter **padap, const char *name);
+extern int dvb_register_adapter (struct dvb_adapter **padap, 
+				 const char *name,
+				 struct module *module);
+
 extern int dvb_unregister_adapter (struct dvb_adapter *adap);
 
 extern int dvb_register_device (struct dvb_adapter *adap,
@@ -92,6 +96,7 @@
 
 extern int dvb_generic_open (struct inode *inode, struct file *file);
 extern int dvb_generic_release (struct inode *inode, struct file *file);
+extern void dvb_device_release(struct dvb_device *dvbdev);
 extern int dvb_generic_ioctl (struct inode *inode, struct file *file,
 			      unsigned int cmd, unsigned long arg);
 #endif /* #ifndef _DVBDEV_H_ */
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/av7110.c dvb-test/linux/drivers/media/dvb/ttpci/av7110.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/av7110.c	2003-09-06 13:05:25.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/av7110.c	2003-09-16 20:51:25.000000000 +0100
@@ -4414,7 +4414,7 @@
 	}
 
 	av7110->dev=(struct saa7146_dev *)dev;
-	dvb_register_adapter(&av7110->dvb_adapter, av7110->card_name);
+	dvb_register_adapter(&av7110->dvb_adapter, av7110->card_name, THIS_MODULE);
 
 	/* the Siemens DVB needs this if you want to have the i2c chips
 	   get recognized before the main driver is fully loaded */
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-av.c dvb-test/linux/drivers/media/dvb/ttpci/budget-av.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-av.c	2003-08-08 17:15:32.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget-av.c	2003-09-16 20:51:25.000000000 +0100
@@ -204,7 +204,7 @@
 
 	memset(budget_av, 0, sizeof(struct budget_av));
 
-	if ((err = ttpci_budget_init(&budget_av->budget, dev, info))) {
+	if ((err = ttpci_budget_init(&budget_av->budget, dev, info, THIS_MODULE))) {
 		kfree(budget_av);
 		return err;
 	}
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget.c dvb-test/linux/drivers/media/dvb/ttpci/budget.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget.c	2003-09-06 13:05:25.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget.c	2003-09-16 20:51:25.000000000 +0100
@@ -154,7 +154,7 @@
 
 	DEB_EE(("dev:%p, info:%p, budget:%p\n",dev,info,budget));
 
-	if ((err = ttpci_budget_init (budget, dev, info))) {
+	if ((err = ttpci_budget_init (budget, dev, info, THIS_MODULE))) {
 		printk("==> failed\n");
 		kfree (budget);
 		return err;
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-ci.c dvb-test/linux/drivers/media/dvb/ttpci/budget-ci.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-ci.c	2003-09-06 13:05:25.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget-ci.c	2003-09-17 23:04:13.000000000 +0100
@@ -325,7 +325,7 @@
 
 	DEB_EE(("budget_ci: %p\n", budget_ci));
 
-	if ((err = ttpci_budget_init (&budget_ci->budget, dev, info))) {
+	if ((err = ttpci_budget_init (&budget_ci->budget, dev, info, THIS_MODULE))) {
 		kfree (budget_ci);
 		return err;
 	}
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-core.c dvb-test/linux/drivers/media/dvb/ttpci/budget-core.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-core.c	2003-09-06 13:05:25.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget-core.c	2003-09-16 20:51:25.000000000 +0100
@@ -198,7 +198,8 @@
 
 int ttpci_budget_init (struct budget *budget,
 		       struct saa7146_dev* dev,
-		       struct saa7146_pci_extension_data *info)
+		       struct saa7146_pci_extension_data *info,
+		       struct module *module)
 {
 	int length = TS_WIDTH*TS_HEIGHT;
 	int ret = 0;
@@ -211,7 +212,7 @@
 	budget->card = bi;
 	budget->dev = (struct saa7146_dev *) dev;
 
-	dvb_register_adapter(&budget->dvb_adapter, budget->card->name);
+	dvb_register_adapter(&budget->dvb_adapter, budget->card->name, module);
 
 	/* set dd1 stream a & b */
       	saa7146_write(dev, DD1_STREAM_B, 0x00000000);
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget.h dvb-test/linux/drivers/media/dvb/ttpci/budget.h
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget.h	2003-06-07 12:53:59.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget.h	2003-09-16 20:51:25.000000000 +0100
@@ -77,7 +77,9 @@
 
 extern int ttpci_budget_init (struct budget *budget,
 			      struct saa7146_dev* dev,
-			      struct saa7146_pci_extension_data *info);
+			      struct saa7146_pci_extension_data *info,
+			      struct module *module);
+
 extern int ttpci_budget_deinit (struct budget *budget);
 extern void ttpci_budget_irq10_handler (struct saa7146_dev* dev, u32 *isr);
 
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-patch.c dvb-test/linux/drivers/media/dvb/ttpci/budget-patch.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttpci/budget-patch.c	2003-09-11 17:35:55.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttpci/budget-patch.c	2003-09-16 20:51:25.000000000 +0100
@@ -172,7 +172,7 @@
 
         DEB_EE(("budget: %p\n",budget));
 
-        if ((err = ttpci_budget_init (budget, dev, info))) {
+        if ((err = ttpci_budget_init (budget, dev, info, THIS_MODULE))) {
                 kfree (budget);
                 return err;
         }
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c dvb-test/linux/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c	2003-09-11 17:35:55.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c	2003-09-16 20:51:25.000000000 +0100
@@ -1129,7 +1129,8 @@
 	up(&ttusb->sem);
 
 	dvb_register_adapter(&ttusb->adapter,
-			     "Technotrend/Hauppauge Nova-USB");
+			     "Technotrend/Hauppauge Nova-USB", 
+			     THIS_MODULE);
 
 	dvb_register_i2c_bus(ttusb_i2c_xfer, ttusb, ttusb->adapter, 0);
 	dvb_add_frontend_ioctls(ttusb->adapter, ttusb_lnb_ioctl, NULL,
diff -ur dvb-kernel-cvs/linux/drivers/media/dvb/ttusb-dec/ttusb_dec.c dvb-test/linux/drivers/media/dvb/ttusb-dec/ttusb_dec.c
--- dvb-kernel-cvs/linux/drivers/media/dvb/ttusb-dec/ttusb_dec.c	2003-09-11 17:35:56.000000000 +0100
+++ dvb-test/linux/drivers/media/dvb/ttusb-dec/ttusb_dec.c	2003-09-16 20:51:25.000000000 +0100
@@ -785,7 +785,7 @@
 
 	dprintk("%s\n", __FUNCTION__);
 
-	if ((result = dvb_register_adapter(&dec->adapter, "dec2000")) < 0) {
+	if ((result = dvb_register_adapter(&dec->adapter, "dec2000", THIS_MODULE)) < 0) {
 		printk("%s: dvb_register_adapter failed: error %d\n",
 		       __FUNCTION__, result);
 

Home | Main Index | Thread Index