Skip to contentSkip to conversation

Adaptec AACRAID: convert fibctx->wait_sem to completion

PATCH In review Matthias Kaehlcke 3 participants 4 messages Feb 13, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Adaptec AACRAID: convert fibctx->wait_sem to completion

Adaptec AACRAID driver: The semaphore fibctx->wait_sem is used for signalling an event. Convert it to a completion.

drivers/scsi/aacraid/aacraid.h +1 -1
index 3195d29..eef4939 100644
@@ -717,7 +717,7 @@ struct aac_fib_context {
 	u32			unique;		// unique value representing this context
 	ulong			jiffies;	// used for cleanup - dmb changed to ulong
 	struct list_head	next;		// used to link context's into a linked list
-	struct semaphore	wait_sem;	// this is used to wait for the next fib to arrive.
+	struct completion	compl;		// this is used to wait for the next fib to arrive.
 	int			wait;		// Set to true when thread is in WaitForSingleObject
 	unsigned long		count;		// total number of FIBs on FibList
 	struct list_head	fib_list;	// this holds fibs and their attachd hw_fibs
drivers/scsi/aacraid/commctrl.c +4 -3
index abef051..98efda6 100644
@@ -33,6 +33,7 @@
 #include <linux/types.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
+#include <linux/completion.h>
 #include <linux/slab.h>
 #include <linux/completion.h>
 #include <linux/dma-mapping.h>
@@ -177,9 +178,9 @@ static int open_getadapter_fib(struct aac_dev * dev, void __user *arg)
 		 */
 		fibctx->unique = (u32)((ulong)fibctx & 0xFFFFFFFF);
 		/*
-		 *	Initialize the mutex used to wait for the next AIF.
+		 *	Initialize the completion used to wait for the next AIF.
 		 */
-		init_MUTEX_LOCKED(&fibctx->wait_sem);
+		init_completion(&fibctx->compl);
 		fibctx->wait = 0;
 		/*
 		 *	Initialize the fibs and set the count of fibs on
@@ -310,7 +311,7 @@ return_fib:
 			ssleep(1);
 		}
 		if (f.wait) {
-			if(down_interruptible(&fibctx->wait_sem) < 0) {
+			if(wait_for_completion_interruptible(&fibctx->compl) < 0) {
 				status = -EINTR;
 			} else {
 				/* Lock again and retry */
drivers/scsi/aacraid/commsup.c +2 -2
index 81b3692..486bdb3 100644
@@ -1440,7 +1440,7 @@ int aac_check_health(struct aac_dev * aac)
 			 * Set the event to wake up the
 			 * thread that will waiting.
 			 */
-			up(&fibctx->wait_sem);
+			complete(&fibctx->compl);
 		} else {
 			printk(KERN_WARNING "aifd: didn't allocate NewFib.\n");
 			kfree(fib);
@@ -1671,7 +1671,7 @@ int aac_command_thread(void *data)
 						 * Set the event to wake up the
 						 * thread that is waiting.
 						 */
-						up(&fibctx->wait_sem);
+						complete(&fibctx->compl);
 					} else {
 						printk(KERN_WARNING "aifd: didn't allocate NewFib.\n");
 					}
Signature · 10 lines
Matthias Kaehlcke
Linux System Developer
Barcelona

  Si deseas mantener tu libertad, debes estar preparado para defenderla
                          (Richard Stallman)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
#1 lore raw

linux/completion is already included in commctrl.c, in the patch fragment it is noted two lines below the addition. Please explain in the description the need/advantages for the change from semaphore to completion.

Sincerely -- Mark Salyzyn

Quoted 118 lines

-----Original Message-----
From: Matthias Kaehlcke [mailto:[email protected]]
Sent: Wednesday, February 13, 2008 1:35 PM
To: AACRAID; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH] Adaptec AACRAID: convert fibctx->wait_sem to completion

Adaptec AACRAID driver: The semaphore fibctx->wait_sem is used for signalling an event. Convert it to a completion.

--

drivers/scsi/aacraid/aacraid.h +0 -0
index 3195d29..eef4939 100644
@@ -717,7 +717,7 @@ struct aac_fib_context {
        u32                     unique;         // unique

value representing this context
ulong jiffies; // used for
cleanup - dmb changed to ulong
struct list_head next; // used to
link context's into a linked list
- struct semaphore wait_sem; // this is
used to wait for the next fib to arrive.
+ struct completion compl; // this is
used to wait for the next fib to arrive.
int wait; // Set to
true when thread is in WaitForSingleObject
unsigned long count; // total
number of FIBs on FibList
struct list_head fib_list; // this holds
fibs and their attachd hw_fibs

drivers/scsi/aacraid/commctrl.c +1 -1
index abef051..98efda6 100644
@@ -33,6 +33,7 @@
 #include <linux/types.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
+#include <linux/completion.h>
 #include <linux/slab.h>
 #include <linux/completion.h>
 #include <linux/dma-mapping.h>
@@ -177,9 +178,9 @@ static int open_getadapter_fib(struct
 aac_dev * dev, void __user *arg)
                 */
                fibctx->unique = (u32)((ulong)fibctx & 0xFFFFFFFF);
                /*
-                *      Initialize the mutex used to wait for

the next AIF.
+ * Initialize the completion used to
wait for the next AIF.

                 */
drivers/scsi/aacraid/commctrl.c +1 -1
-               init_MUTEX_LOCKED(&fibctx->wait_sem);
+               init_completion(&fibctx->compl);
                fibctx->wait = 0;
                /*
                 *      Initialize the fibs and set the count

of fibs on

drivers/scsi/aacraid/commctrl.c +1 -1
@@ -310,7 +311,7 @@ return_fib:
                        ssleep(1);
                }
                if (f.wait) {
-
 if(down_interruptible(&fibctx->wait_sem) < 0) {
+
 if(wait_for_completion_interruptible(&fibctx->compl) < 0) {
                                status = -EINTR;
                        } else {
                                /* Lock again and retry */
drivers/scsi/aacraid/commsup.c +1 -1
index 81b3692..486bdb3 100644
@@ -1440,7 +1440,7 @@ int aac_check_health(struct aac_dev * aac)
                         * Set the event to wake up the
                         * thread that will waiting.
                         */
-                       up(&fibctx->wait_sem);
+                       complete(&fibctx->compl);
                } else {
                        printk(KERN_WARNING "aifd: didn't
 allocate NewFib.\n");
                        kfree(fib);
drivers/scsi/aacraid/commsup.c +0 -0
@@ -1671,7 +1671,7 @@ int aac_command_thread(void *data)
                                                 * Set the

event to wake up the
* thread
that is waiting.

                                                 */
drivers/scsi/aacraid/commsup.c +1 -1
-                                               up(&fibctx->wait_sem);
+
complete(&fibctx->compl);
                                        } else {
printk(KERN_WARNING "aifd: didn't allocate NewFib.\n");
                                        }

--
Matthias Kaehlcke
Linux System Developer
Barcelona

Si deseas mantener tu libertad, debes estar preparado para
defenderla
(Richard Stallman)
.''`.
using free software / Debian GNU/Linux |
http://debian.org : :' :

                                                                `. `'`

gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-

maintainer
#2 lore raw
In reply to Matthias Kaehlcke
Quoted 2 lines · Matthias Kaehlcke

Adaptec AACRAID driver: The semaphore fibctx->wait_sem is used for signalling an event. Convert it to a completion.

What's the purpose of doing this? The aacraid implementation seems to be a classic PV semaphore, so why does it need to become a completion?

James

#3 lore raw

El Wed, Feb 13, 2008 at 01:09:00PM -0600 James Bottomley ha dit:

Quoted 6 lines · Matthias Kaehlcke
Quoted 2 lines

Adaptec AACRAID driver: The semaphore fibctx->wait_sem is used for signalling an event. Convert it to a completion.

What's the purpose of doing this? The aacraid implementation seems to be a classic PV semaphore, so why does it need to become a completion?

i had another look at the code and i agree with you. i didn't realize that the semaphore is used as counting semaphore

please redirect the patch to /dev/null

thanks for reviewing the patch and pointing out my mistake

Signature · 12 lines
Matthias Kaehlcke
Linux System Developer
Barcelona

      In itself, homosexuality is as limiting as heterosexuality: the
     ideal should be to be capable of loving a woman or a man; either,
      a human being, without  feeling fear, restraint, or obligation
                          (Simone de Beauvoir)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-