[PATCH 1/1] aacraid: fib context lock for management ioctls (take 2)

2008-01-28 Thread Salyzyn, Mark
The first patch was a bit too aggressive and nested the locks (!) unit testing 
was in error.

Signed-off-by: Mark Salyzyn [EMAIL PROTECTED]

 drivers/scsi/aacraid/commctrl.c |   26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

Sincerely -- Mark Salyzyn

 -Original Message-
 From: [EMAIL PROTECTED]
 [mailto:[EMAIL PROTECTED] On Behalf Of Salyzyn, Mark
 Sent: Thursday, January 24, 2008 10:40 AM
 To: 'linux-scsi@vger.kernel.org'
 Cc: 'Alan Cox'; 'Alan Cox'
 Subject: [PATCH 1/1] aacraid: fib context lock for management ioctls

 Alan noticed the lack of locking surrounding the driver's
 dealings with the fib context managed by the trio of ioctls
 that are used by the RAID management applications to retrieve
 Adapter Initiated FIBs. I merely expanded the fib lock to
 include the fib context. There have been no field reports of
 any issues generally because the applications are relatively
 static and do not come and go often enough to stress this
 area. I bloated this patch a little with some space junk.

 This attached patch is against current scsi-misc-2.6.

 ObligatoryDisclaimer: Please accept my condolences regarding
 Outlook's handling of patch attachments. The following inline
 patch is 'diff -rub' to pull out the space junk to enable
 convenient inspection, please use the attached file to patch.

 Signed-off-by: Mark Salyzyn [EMAIL PROTECTED]

  drivers/scsi/aacraid/commctrl.c |   29 +
  1 file changed, 17 insertions(+), 12 deletions(-)

 diff -rub a/commctrl.c b/commctrl.c
 --- a/drivers/scsi/aacraid/commctrl.c2008-01-24
 09:44:33.080806785 -0500
 +++ b/drivers/scsi/aacraid/commctrl.c2008-01-24
 09:50:41.071552674 -0500
 @@ -243,6 +243,7 @@
  *  Search the list of AdapterFibContext
 addresses on the adapter
  *  to be sure this is a valid address
  */
 +   spin_lock_irqsave(dev-fib_lock, flags);
 entry = dev-fib_list.next;
 fibctx = NULL;

 @@ -258,17 +259,18 @@
 fibctx = NULL;
 }
 if (!fibctx) {
 +   spin_unlock_irqrestore(dev-fib_lock, flags);
 dprintk ((KERN_INFO Fib Context not found\n));
 return -EINVAL;
 }

 if((fibctx-type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
  (fibctx-size != sizeof(struct aac_fib_context))) {
 +   spin_unlock_irqrestore(dev-fib_lock, flags);
 dprintk ((KERN_INFO Fib Context corrupt?\n));
 return -EINVAL;
 }
 status = 0;
 -   spin_lock_irqsave(dev-fib_lock, flags);
 /*
  *  If there are no fibs to send back, then
 either wait or return
  *  -EAGAIN

Here is the offending portion of the previous patch (two chunks):

 @@ -326,7 +328,9 @@
  int aac_close_fib_context(struct aac_dev * dev, struct
 aac_fib_context * fibctx)
  {
 struct fib *fib;
 +   unsigned long flags;

 +   spin_lock_irqsave(dev-fib_lock, flags);
 /*
  *  First free any FIBs that have not been consumed.
  */
 @@ -349,6 +353,7 @@
  *  Remove the Context from the AdapterFibContext List
  */
 list_del(fibctx-next);
 +   spin_unlock_irqrestore(dev-fib_lock, flags);
 /*
  *  Invalidate context
  */

 Sincerely - Mark Salyzyn


aacraid_fibctx_lock2.patch
Description: aacraid_fibctx_lock2.patch


[PATCH 1/1] aacraid: fib context lock for management ioctls

2008-01-24 Thread Salyzyn, Mark
Alan noticed the lack of locking surrounding the driver's dealings with the fib 
context managed by the trio of ioctls that are used by the RAID management 
applications to retrieve Adapter Initiated FIBs. I merely expanded the fib lock 
to include the fib context. There have been no field reports of any issues 
generally because the applications are relatively static and do not come and go 
often enough to stress this area. I bloated this patch a little with some space 
junk.

This attached patch is against current scsi-misc-2.6.

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling 
of patch attachments. The following inline patch is 'diff -rub' to pull out the 
space junk to enable convenient inspection, please use the attached file to 
patch.

Signed-off-by: Mark Salyzyn [EMAIL PROTECTED]

 drivers/scsi/aacraid/commctrl.c |   29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff -rub a/commctrl.c b/commctrl.c
--- a/drivers/scsi/aacraid/commctrl.c2008-01-24 09:44:33.080806785 -0500
+++ b/drivers/scsi/aacraid/commctrl.c2008-01-24 09:50:41.071552674 -0500
@@ -243,6 +243,7 @@
 *  Search the list of AdapterFibContext addresses on the adapter
 *  to be sure this is a valid address
 */
+   spin_lock_irqsave(dev-fib_lock, flags);
entry = dev-fib_list.next;
fibctx = NULL;

@@ -258,17 +259,18 @@
fibctx = NULL;
}
if (!fibctx) {
+   spin_unlock_irqrestore(dev-fib_lock, flags);
dprintk ((KERN_INFO Fib Context not found\n));
return -EINVAL;
}

if((fibctx-type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
 (fibctx-size != sizeof(struct aac_fib_context))) {
+   spin_unlock_irqrestore(dev-fib_lock, flags);
dprintk ((KERN_INFO Fib Context corrupt?\n));
return -EINVAL;
}
status = 0;
-   spin_lock_irqsave(dev-fib_lock, flags);
/*
 *  If there are no fibs to send back, then either wait or return
 *  -EAGAIN
@@ -326,7 +328,9 @@
 int aac_close_fib_context(struct aac_dev * dev, struct aac_fib_context * 
fibctx)
 {
struct fib *fib;
+   unsigned long flags;

+   spin_lock_irqsave(dev-fib_lock, flags);
/*
 *  First free any FIBs that have not been consumed.
 */
@@ -349,6 +353,7 @@
 *  Remove the Context from the AdapterFibContext List
 */
list_del(fibctx-next);
+   spin_unlock_irqrestore(dev-fib_lock, flags);
/*
 *  Invalidate context
 */

Sincerely - Mark Salyzyn


aacraid_fibctx_lock.patch
Description: aacraid_fibctx_lock.patch