[cpia] [PATCH] Updated locking for CPiA driver

[David C. Hansen]

I added a few things to the CPiA driver.   First, I modified the
cam_data struct to use list_heads instead of its own list members.  This
allowed the removal of the #defined list routines.

I also replaced the use of the BKL in the driver.  I gave it 1 lock for
USB and one for PP.  There was a small potential race condition in the
pp_detach function.  The list removal itself was protected by the BKL,
but the iteration through the list in the for loop was not. 

--
David C. Hansen
haveblue@us.ibm.com
IBM LTC Base/OS Group
(503)578-4080

--- ../clean/linux/drivers/media/video/cpia.h	Fri Mar  2 11:12:10 2001
+++ linux/drivers/media/video/cpia.h	Fri Sep 14 11:08:03 2001
@@ -226,8 +226,7 @@
 #define FRAME_NUM	2	/* double buffering for now */
 
 struct cam_data {
-	struct cam_data **previous;
-	struct cam_data *next;
+	struct list_head cam_list;
 
         struct semaphore busy_lock;     /* guard against SMP
multithreading */
 	struct cpia_camera_ops *ops;	/* lowlevel driver operations */
@@ -392,29 +391,6 @@
   DBG("%1d %1d %1d %1d %1d %1d %1d %1d \n",\
       (p)&0x80?1:0, (p)&0x40?1:0, (p)&0x20?1:0, (p)&0x10?1:0,\
         (p)&0x08?1:0, (p)&0x04?1:0, (p)&0x02?1:0, (p)&0x01?1:0);
-
-#define ADD_TO_LIST(l, drv) \
-  {\
-    lock_kernel();\
-    (drv)->next = l;\
-    (drv)->previous = &(l);\
-    (l) = drv;\
-    unlock_kernel();\
-  } while(0)
-
-#define REMOVE_FROM_LIST(drv) \
-  {\
-    if ((drv)->previous != NULL) {\
-      lock_kernel();\
-      if ((drv)->next != NULL)\
-        (drv)->next->previous = (drv)->previous;\
-      *((drv)->previous) = (drv)->next;\
-      (drv)->previous = NULL;\
-      (drv)->next = NULL;\
-      unlock_kernel();\
-    }\
-  } while (0)
-
 
 #endif /* __KERNEL__ */
 
--- ../clean/linux/drivers/media/video/cpia_pp.c	Fri Mar  2 11:12:10
2001
+++ linux/drivers/media/video/cpia_pp.c	Fri Sep 14 11:43:05 2001
@@ -157,7 +157,8 @@
 	1
 };
 
-static struct cam_data *cam_list;
+LIST_HEAD(cam_list_head);
+static spinlock_t cam_list_lock_pp = SPIN_LOCK_UNLOCKED;
 
 #ifdef _CPIA_DEBUG_
 #define DEB_PORT(port) { \
@@ -582,20 +583,24 @@
 		kfree(cam);
 		return -ENXIO;
 	}
-	ADD_TO_LIST(cam_list, cpia);
+	spin_lock( &cam_list_lock_pp );
+	list_add(&cpia->cam_list, &cam_list_head);
+	spin_unlock( &cam_list_lock_pp );
 
 	return 0;
 }
 
 static void cpia_pp_detach (struct parport *port)
 {
-	struct cam_data *cpia;
+	struct list_head* pos;
 
-	for(cpia = cam_list; cpia != NULL; cpia = cpia->next) {
+	spin_lock( &cam_list_lock_pp );
+	list_for_each(pos, &cam_list_head) {
+		struct cam_data *cpia = list_entry( pos, struct cam_data, cam_list );
 		struct pp_cam_entry *cam = cpia->lowlevel_data;
 		if (cam && cam->port->number == port->number) {
-			REMOVE_FROM_LIST(cpia);
-			
+		        list_del(pos);
+			spin_unlock( &cam_list_lock_pp );			
 			cpia_unregister_camera(cpia);
 			
 			if(cam->open_count > 0) {
@@ -606,9 +611,10 @@
 		
 			kfree(cam);
 			cpia->lowlevel_data = NULL;
-			break;
+			return;
 		}
 	}
+	spin_unlock( &cam_list_lock_pp );
 }
 
 static void cpia_pp_attach (struct parport *port)
@@ -660,7 +666,7 @@
 		LOG ("unable to register with parport\n");
 		return -EIO;
 	}
-
+	
 	return 0;
 }
 
--- ../clean/linux/drivers/media/video/cpia_usb.c	Mon Feb 19 10:18:18
2001
+++ linux/drivers/media/video/cpia_usb.c	Fri Sep 14 11:44:36 2001
@@ -103,7 +103,8 @@
 	0
 };
 
-static struct cam_data *cam_list;
+LIST_HEAD(cam_list_head);
+static spinlock_t cam_list_lock_usb = SPIN_LOCK_UNLOCKED;
 
 static void cpia_usb_complete(struct urb *urb)
 {
@@ -536,7 +537,9 @@
 		goto fail_all;
 	}
 
-	ADD_TO_LIST(cam_list, cam);
+	spin_lock( &cam_list_lock_usb );
+        list_add(&cam->cam_list,&cam_list_head);
+	spin_unlock( &cam_list_lock_usb );
 
 	return cam;
 
@@ -579,8 +582,10 @@
 	struct cam_data *cam = (struct cam_data *) ptr;
 	struct usb_cpia *ucpia = (struct usb_cpia *) cam->lowlevel_data;
   
-	REMOVE_FROM_LIST(cam);
-
+	spin_lock( &cam_list_lock_usb );
+	list_del(&cam->cam_list);
+	spin_unlock( &cam_list_lock_usb );
+	
 	/* Don't even try to reset the altsetting if we're disconnected */
 	cpia_usb_free_resources(ucpia, 0);
 
@@ -619,8 +624,6 @@
 
 static int __init usb_cpia_init(void)
 {
-	cam_list = NULL;
-
 	return usb_register(&cpia_driver);
 }