public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH, 2.6] tmscsim - no internal command-queuing
@ 2004-01-30 23:23 Guennadi Liakhovetski
  2004-02-01 11:17 ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Guennadi Liakhovetski @ 2004-01-30 23:23 UTC (permalink / raw)
  To: linux-scsi; +Cc: Kurt Garloff

[-- Attachment #1: Type: TEXT/PLAIN, Size: 237 bytes --]

Hi

The attached patch removes the internal command-queuing in tmscsim. The
driver still works:-) Don't know though if this is quite correct. Please,
comment. Goes on the top of my yesterday's patch.

Guennadi
---
Guennadi Liakhovetski


[-- Attachment #2: Type: TEXT/PLAIN, Size: 6771 bytes --]

diff -ur a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c
--- a/drivers/scsi/tmscsim.c	Thu Jan 29 21:45:15 2004
+++ b/drivers/scsi/tmscsim.c	Fri Jan 30 23:59:53 2004
@@ -722,37 +722,6 @@
     return q;
 }
 #endif
-    
-
-/* Append to Query List */
-static void dc390_Query_append( PSCSICMD cmd, PACB pACB )
-{
-	dc390_cmd_scp_t *cmdq = (dc390_cmd_scp_t *)&cmd->SCp;
-
-	DEBUG0(printk ("DC390: Append cmd %li to Query\n", cmd->pid));
-
-	list_add_tail(&cmdq->list, &pACB->cmdq);
-	pACB->QueryCnt++;
-	pACB->CmdOutOfSRB++;
-}
-
-
-/* Return next cmd from Query list */
-static PSCSICMD dc390_Query_get ( PACB pACB )
-{
-	PSCSICMD  pcmd;
-	dc390_cmd_scp_t *cmdq;
-	if (list_empty(&pACB->cmdq))
-		return NULL;
-
-	pcmd = (PSCSICMD) list_entry(pACB->cmdq.next, struct scsi_cmnd_list, scp.list);
-	DEBUG0(printk ("DC390: Get cmd %li from Query\n", pcmd->pid));
-	cmdq = (dc390_cmd_scp_t *)&pcmd->SCp;
-	list_del(&cmdq->list);
-	pACB->QueryCnt--;
-	return pcmd;
-}
-
 
 /* Return next free SRB */
 static __inline__ PSRB dc390_Free_get ( PACB pACB )
@@ -1074,38 +1043,6 @@
     /* KG: deferred PCI mapping to dc390_StartSCSI */
 }
 
-/* Put cmnd from Query to Waiting list and send next Waiting cmnd */
-static void dc390_Query_to_Waiting (PACB pACB)
-{
-    Scsi_Cmnd *pcmd;
-    PSRB   pSRB;
-    PDCB   pDCB;
-
-    if( pACB->ACBFlag & (RESET_DETECT+RESET_DONE+RESET_DEV) )
-	return;
-
-    while (pACB->QueryCnt)
-    {
-	pSRB = dc390_Free_get ( pACB );
-	if (!pSRB) return;
-	pcmd = dc390_Query_get ( pACB );
-	if (!pcmd) { dc390_Free_insert (pACB, pSRB); return; } /* should not happen */
-	pDCB = dc390_findDCB (pACB, pcmd->device->id, pcmd->device->lun);
-	if (!pDCB) 
-	{ 
-		dc390_Free_insert (pACB, pSRB);
-		printk (KERN_ERR "DC390: Command in queue to non-existing device!\n");
-		pcmd->result = MK_RES(DRIVER_ERROR,DID_ERROR,0,0);
-		DC390_UNLOCK_ACB_NI;
-		pcmd->done (pcmd);
-		DC390_LOCK_ACB_NI;
-		return;
-	}
-	dc390_BuildSRB (pcmd, pDCB, pSRB);
-	dc390_Waiting_append ( pDCB, pSRB );
-    }
-}
-
 /***********************************************************************
  * Function : static int DC390_queue_command (Scsi_Cmnd *cmd,
  *					       void (*done)(Scsi_Cmnd *))
@@ -1214,20 +1151,12 @@
     pACB->Cmds++;
     cmd->scsi_done = done;
     cmd->result = 0;
-	
-    dc390_Query_to_Waiting (pACB);
-
-    if( pACB->QueryCnt ) /* Unsent commands ? */
-    {
-	DEBUG0(printk ("DC390: QueryCnt != 0\n"));
-	dc390_Query_append ( cmd, pACB );
-	dc390_Waiting_process (pACB);
-    }
-    else if (pDCB->pWaitingSRB)
+    if (pDCB->pWaitingSRB)
     {
  	pSRB = dc390_Free_get ( pACB );
 	DEBUG0(if (!pSRB) printk ("DC390: No free SRB but Waiting\n"); else printk ("DC390: Free SRB w/ Waiting\n"));
-	if (!pSRB) dc390_Query_append (cmd, pACB);
+	if (!pSRB)
+	    return 1;
 	else 
 	  {
 	    dc390_BuildSRB (cmd, pDCB, pSRB);
@@ -1241,8 +1170,8 @@
 	DEBUG0(if (!pSRB) printk ("DC390: No free SRB w/o Waiting\n"); else printk ("DC390: Free SRB w/o Waiting\n"));
 	if (!pSRB)
 	{
-	    dc390_Query_append (cmd, pACB);
 	    dc390_Waiting_process (pACB);
+	    return 1;
 	}
 	else 
 	{
@@ -1460,21 +1389,6 @@
     printk ("DC390: Abort command (pid %li, Device %02i-%02i)\n",
 	    cmd->pid, cmd->device->id, cmd->device->lun);
 
-    /* First scan Query list */
-    if( pACB->QueryCnt )
-    {
-	struct scsi_cmnd_list *t, *pcmd_l;
-	list_for_each_entry_safe(pcmd_l, t, &pACB->cmdq, scp.list)
-		if( (struct scsi_cmnd*)pcmd_l == cmd )
-		{
-			/* Found: Dequeue */
-			list_del(&pcmd_l->scp.list);
-			pACB->QueryCnt--;
-			status = SCSI_ABORT_SUCCESS;
-			goto  ABO_X;
-		}
-    }
-	
     pDCB = dc390_findDCB (pACB, cmd->device->id, cmd->device->lun);
     if( !pDCB ) goto  NOT_RUN;
 
@@ -1928,8 +1842,6 @@
     pACB->pActiveDCB = NULL;
     pACB->pFreeSRB = pACB->SRB_array;
     pACB->SRBCount = MAX_SRB_CNT;
-    pACB->QueryCnt = 0;
-    INIT_LIST_HEAD(&pACB->cmdq);
     pACB->AdapterIndex = index;
     pACB->status = 0;
     psh->this_id = dc390_eepromBuf[index][EE_ADAPT_SCSI_ID];
@@ -2727,7 +2639,6 @@
 {
   int dev, spd, spd1;
   char *pos = buffer;
-  struct scsi_cmnd_list *cl;
   PACB pACB;
   PDCB pDCB;
   DC390_AFLAGS;
@@ -2798,9 +2709,6 @@
       SPRINTF ("      %02i\n", pDCB->MaxCommand);
       pDCB = pDCB->pNextDCB;
      }
-    SPRINTF ("Commands in Queues: Query: %li:", pACB->QueryCnt);
-    list_for_each_entry(cl, &pACB->cmdq, scp.list)
-	SPRINTF (" %li", ((struct scsi_cmnd*)cl)->pid);
     if (timer_pending(&pACB->Waiting_Timer)) SPRINTF ("Waiting queue timer running\n");
     else SPRINTF ("\n");
     pDCB = pACB->pLinkDCB;
diff -ur a/drivers/scsi/tmscsim.h b/drivers/scsi/tmscsim.h
--- a/drivers/scsi/tmscsim.h	Mon Nov 24 23:47:29 2003
+++ b/drivers/scsi/tmscsim.h	Sat Jan 31 00:04:57 2004
@@ -214,16 +214,12 @@
 PSRB		pTmpSRB;
 
 /* 0x2c: */
-ULONG		QueryCnt;
-struct list_head	cmdq;
-
-/* 0x38: */
 UCHAR		msgin123[4];
 UCHAR		DCBmap[MAX_SCSI_ID];
 UCHAR		Connected;
 UCHAR		pad;
 
-/* 0x3c: */
+/* 0x30: */
 #if defined(USE_SPINLOCKS) && USE_SPINLOCKS > 1 && (defined(CONFIG_SMP) || DEBUG_SPINLOCKS > 0)
 spinlock_t	lock;
 #endif
@@ -234,20 +230,20 @@
 UCHAR		Ignore_IRQ;	/* Not used */
 
 PDEVDECL1;			/* Pointer to PCI cfg. space */
-/* 0x4c/0x48: */
+/* 0x40/0x3c: */
 ULONG		Cmds;
 UINT		SelLost;
 UINT		SelConn;
 UINT		CmdInQ;
 UINT		CmdOutOfSRB;
 	
-/* 0x60/0x5c: */
+/* 0x54/0x50: */
 struct timer_list	Waiting_Timer;
-/* 0x74/0x70: */
+/* 0x68/0x64: */
 DC390_SRB	TmpSRB;
-/* 0xd8/0xd4: */
+/* 0xcc/0xc8: */
 DC390_SRB	SRB_array[MAX_SRB_CNT]; 	/* 50 SRBs */
-/* 0xfb0/0xfac: */
+/* 0xfa4/0xfa0: */
 };
 
 typedef  struct  _ACB	 DC390_ACB, *PACB;
@@ -406,15 +402,8 @@
  *	SISC query queue
  */
 typedef struct {
-	struct list_head	list;
 	dma_addr_t		saved_dma_handle;
 } dc390_cmd_scp_t;
-
-struct scsi_cmnd_list
-{
-	char dummy[offsetof(struct scsi_cmnd, SCp)];
-	dc390_cmd_scp_t scp;
-};
 
 /*
 **  Inquiry Data format
diff -ur a/drivers/scsi/scsiiom.c linux-2.6.1/drivers/scsi/scsiiom.c
--- a/drivers/scsi/scsiiom.c	Sat Nov 29 22:28:25 2003
+++ b/drivers/scsi/scsiiom.c	Sat Jan 31 00:05:45 2004
@@ -1150,7 +1150,6 @@
 	    pSRB = psrb;
 	}
 	pDCB->pGoingSRB = 0;
-	dc390_Query_to_Waiting (pACB);
 	dc390_Waiting_process (pACB);
     }
     else
@@ -1630,7 +1629,6 @@
     pcmd->scsi_done (pcmd);
     DC390_LOCK_ACB_NI;
 
-    dc390_Query_to_Waiting (pACB);
     dc390_Waiting_process (pACB);
     return;
 }
@@ -1677,7 +1675,6 @@
 	pdcb->TagMask = 0;
 	pdcb = pdcb->pNextDCB;
     } while( pdcb != pDCB );
-    dc390_Query_to_Waiting (pACB);
 }
 
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 2.6] tmscsim - no internal command-queuing
  2004-01-30 23:23 [PATCH, 2.6] tmscsim - no internal command-queuing Guennadi Liakhovetski
@ 2004-02-01 11:17 ` Christoph Hellwig
  2004-02-01 15:41   ` James Bottomley
  2004-02-03 22:41   ` Guennadi Liakhovetski
  0 siblings, 2 replies; 6+ messages in thread
From: Christoph Hellwig @ 2004-02-01 11:17 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-scsi, Kurt Garloff

On Sat, Jan 31, 2004 at 12:23:48AM +0100, Guennadi Liakhovetski wrote:
> Hi
> 
> The attached patch removes the internal command-queuing in tmscsim. The
> driver still works:-) Don't know though if this is quite correct. Please,
> comment. Goes on the top of my yesterday's patch.

Patch looks mostly good for me.  But there's some fishyness in queuecommand,
mostly from before you patch:

 - many failure cases return one with the new EH code although we wouldn't
   want to requeue the midlayer in that case.  I removed the ifdef and added
   a failed goto to handle them.
 - we need to set cmd->result onlyh if we have an error instead of always
   an overriding it - else it will leak to the midlayer in the return 1
   case.
 - the check for ids out of range are superflous (this was one of the errors
   above, I decided to remove it instead of fixing it).
 - you don't do DC390_UNLOCK_ACB when failing.  While it's a noop I think
   it's bad to have locking macros in place and don't balance them.  You
   should probably remove it completly in one of the next patches.
 - the pDCB->pWaitingSRB looks a bit strange.  By unifying the the codepathes
   it becomes much more readable.

While I'm looking at queuecommand here's some suggestions for future driver
revisions:

 - dc390_initDCB should move to the slave_alloc callback - you get that once
   allocate scsi device, which is much cleaner.  Then you can store it
   in sdev->hostdata and easily retrieve it in ->queuecommand instead of the
   dc390_findDCB and free it in ->slave_destroy.
 - Above change would probably also get rid of the whole scan device mess,
   but I'd like to hear some words from Kurt on the details of it first.

And here's my patch ontop of your two (completely untested due to lack of
hardware):


--- drivers/scsi/tmscsim.c~	2004-01-31 02:04:47.000000000 +0100
+++ drivers/scsi/tmscsim.c	2004-01-31 02:27:12.000000000 +0100
@@ -1117,9 +1117,6 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
 
     DC390_LOCK_ACB;
     
-    /* Assume BAD_TARGET; will be cleared later */
-    cmd->result = DID_BAD_TARGET << 16;
-   
     /* TODO: Change the policy: Alway accept TEST_UNIT_READY or INQUIRY 
      * commands and alloc a DCB for the device if not yet there. DCB will
      * be removed in dc390_SRBdone if SEL_TIMEOUT */
@@ -1130,17 +1127,6 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
     else if( (pACB->scan_devices) && (cmd->cmnd[0] == READ_6) )
 	pACB->scan_devices = 0;
 
-    if ( ( cmd->device->id >= pACB->pScsiHost->max_id ) || 
-	 (cmd->device->lun >= pACB->pScsiHost->max_lun) )
-    {
-/*	printk ("DC390: Ignore target %d lun %d\n",
-		cmd->device->id, cmd->device->lun); */
-	DC390_UNLOCK_ACB;
-	//return (1);
-	done (cmd);
-	return (0);
-    }
-
     if( (pACB->scan_devices || cmd->cmnd[0] == TEST_UNIT_READY || cmd->cmnd[0] == INQUIRY) && 
        !(pACB->DCBmap[cmd->device->id] & (1 << cmd->device->lun)) )
     {
@@ -1151,14 +1137,7 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
 	  {
 	    printk (KERN_ERR "DC390: kmalloc for DCB failed, target %02x lun %02x\n", 
 		    cmd->device->id, cmd->device->lun);
-	    DC390_UNLOCK_ACB;
-	    printk ("DC390: No DCB in queue_command!\n");
-#ifdef USE_NEW_EH
-	    return (1);
-#else
-	    done (cmd);
-	    return (0);
-#endif
+	    goto fail;
 	  }
             
     }
@@ -1166,10 +1145,7 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
     {
 	printk(KERN_INFO "DC390: Ignore target %02x lun %02x\n",
 		cmd->device->id, cmd->device->lun); 
-	DC390_UNLOCK_ACB;
-	//return (1);
-	done (cmd);
-	return (0);
+	goto fail;
     }
     else
     {
@@ -1178,52 +1154,37 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
 	 {  /* should never happen */
 	    printk (KERN_ERR "DC390: no DCB failed, target %02x lun %02x\n", 
 		    cmd->device->id, cmd->device->lun);
-	    DC390_UNLOCK_ACB;
-	    printk ("DC390: No DCB in queuecommand (2)!\n");
-#ifdef USE_NEW_EH
-	    return (1);
-#else
-	    done (cmd);
-	    return (0);
-#endif
+	    goto fail;
 	 }
     }
 
     pACB->Cmds++;
     cmd->scsi_done = done;
     cmd->result = 0;
-    if (pDCB->pWaitingSRB)
-    {
- 	pSRB = dc390_Free_get ( pACB );
-	DEBUG0(if (!pSRB) printk ("DC390: No free SRB but Waiting\n"); else printk ("DC390: Free SRB w/ Waiting\n"));
-	if (!pSRB)
-	    return 1;
-	else 
-	  {
-	    dc390_BuildSRB (cmd, pDCB, pSRB);
-	    dc390_Waiting_append (pDCB, pSRB);
-	  }
-	dc390_Waiting_process (pACB);
-    }
-    else
-    {
- 	pSRB = dc390_Free_get ( pACB );
-	DEBUG0(if (!pSRB) printk ("DC390: No free SRB w/o Waiting\n"); else printk ("DC390: Free SRB w/o Waiting\n"));
-	if (!pSRB)
-	{
-	    dc390_Waiting_process (pACB);
-	    return 1;
-	}
-	else 
-	{
-	    dc390_BuildSRB (cmd, pDCB, pSRB);
-	    dc390_SendSRB (pACB, pSRB);
-	}
-    }
+
+    pSRB = dc390_Free_get(pACB);
+    if (!pSRB)
+	    goto requeue;
+
+    dc390_BuildSRB(cmd, pDCB, pSRB);
+    if (pDCB->pWaitingSRB) {
+	    dc390_Waiting_append(pDCB, pSRB);
+	    dc390_Waiting_process(pACB);
+    } else
+	    dc390_SendSRB(pACB, pSRB);
 
     DC390_UNLOCK_ACB;
     DEBUG1(printk (KERN_DEBUG " ... command (pid %li) queued successfully.\n", cmd->pid));
     return(0);
+
+ requeue:
+    DC390_UNLOCK_ACB;
+    return 1;
+ fail:
+    DC390_UNLOCK_ACB;
+    cmd->result = DID_BAD_TARGET << 16;
+    done(cmd);
+    return 0;
 }
 
 /* We ignore mapping problems, as we expect everybody to respect 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 2.6] tmscsim - no internal command-queuing
  2004-02-01 11:17 ` Christoph Hellwig
@ 2004-02-01 15:41   ` James Bottomley
  2004-02-03 22:41   ` Guennadi Liakhovetski
  1 sibling, 0 replies; 6+ messages in thread
From: James Bottomley @ 2004-02-01 15:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Guennadi Liakhovetski, SCSI Mailing List, Kurt Garloff

On Sun, 2004-02-01 at 06:17, Christoph Hellwig wrote:
> +
> + requeue:
> +    DC390_UNLOCK_ACB;
> +    return 1;

Since this is a host pausing requeue, the return value should be
SCSI_MLQUEUE_HOST_BUSY.

James



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 2.6] tmscsim - no internal command-queuing
  2004-02-01 11:17 ` Christoph Hellwig
  2004-02-01 15:41   ` James Bottomley
@ 2004-02-03 22:41   ` Guennadi Liakhovetski
  2004-02-19 15:13     ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Guennadi Liakhovetski @ 2004-02-03 22:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-scsi, Kurt Garloff

On Sun, 1 Feb 2004, Christoph Hellwig wrote:

> Patch looks mostly good for me.  But there's some fishyness in queuecommand,
> mostly from before you patch:
>
>  - many failure cases return one with the new EH code although we wouldn't
>    want to requeue the midlayer in that case.  I removed the ifdef and added
>    a failed goto to handle them.
>  - we need to set cmd->result onlyh if we have an error instead of always
>    an overriding it - else it will leak to the midlayer in the return 1
>    case.
>  - the check for ids out of range are superflous (this was one of the errors
>    above, I decided to remove it instead of fixing it).
>  - you don't do DC390_UNLOCK_ACB when failing.  While it's a noop I think
>    it's bad to have locking macros in place and don't balance them.  You
>    should probably remove it completly in one of the next patches.

Ouch. Whereas the rest are either rather intelligent things I didn't know
about, or something I didn't want / dare to modify without a burning need,
this is definitely just a plain simple bug - I did see it was a noop, and
I did think at some point to preserve its consistency, until I forgot
about it:-)

Thanks
Guennadi

>  - the pDCB->pWaitingSRB looks a bit strange.  By unifying the the codepathes
>    it becomes much more readable.
>
> While I'm looking at queuecommand here's some suggestions for future driver
> revisions:
>
>  - dc390_initDCB should move to the slave_alloc callback - you get that once
>    allocate scsi device, which is much cleaner.  Then you can store it
>    in sdev->hostdata and easily retrieve it in ->queuecommand instead of the
>    dc390_findDCB and free it in ->slave_destroy.
>  - Above change would probably also get rid of the whole scan device mess,
>    but I'd like to hear some words from Kurt on the details of it first.
>
> And here's my patch ontop of your two (completely untested due to lack of
> hardware):
>
>
> --- drivers/scsi/tmscsim.c~	2004-01-31 02:04:47.000000000 +0100
> +++ drivers/scsi/tmscsim.c	2004-01-31 02:27:12.000000000 +0100
> @@ -1117,9 +1117,6 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
>
>      DC390_LOCK_ACB;
>
> -    /* Assume BAD_TARGET; will be cleared later */
> -    cmd->result = DID_BAD_TARGET << 16;
> -
>      /* TODO: Change the policy: Alway accept TEST_UNIT_READY or INQUIRY
>       * commands and alloc a DCB for the device if not yet there. DCB will
>       * be removed in dc390_SRBdone if SEL_TIMEOUT */
> @@ -1130,17 +1127,6 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
>      else if( (pACB->scan_devices) && (cmd->cmnd[0] == READ_6) )
>  	pACB->scan_devices = 0;
>
> -    if ( ( cmd->device->id >= pACB->pScsiHost->max_id ) ||
> -	 (cmd->device->lun >= pACB->pScsiHost->max_lun) )
> -    {
> -/*	printk ("DC390: Ignore target %d lun %d\n",
> -		cmd->device->id, cmd->device->lun); */
> -	DC390_UNLOCK_ACB;
> -	//return (1);
> -	done (cmd);
> -	return (0);
> -    }
> -
>      if( (pACB->scan_devices || cmd->cmnd[0] == TEST_UNIT_READY || cmd->cmnd[0] == INQUIRY) &&
>         !(pACB->DCBmap[cmd->device->id] & (1 << cmd->device->lun)) )
>      {
> @@ -1151,14 +1137,7 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
>  	  {
>  	    printk (KERN_ERR "DC390: kmalloc for DCB failed, target %02x lun %02x\n",
>  		    cmd->device->id, cmd->device->lun);
> -	    DC390_UNLOCK_ACB;
> -	    printk ("DC390: No DCB in queue_command!\n");
> -#ifdef USE_NEW_EH
> -	    return (1);
> -#else
> -	    done (cmd);
> -	    return (0);
> -#endif
> +	    goto fail;
>  	  }
>
>      }
> @@ -1166,10 +1145,7 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
>      {
>  	printk(KERN_INFO "DC390: Ignore target %02x lun %02x\n",
>  		cmd->device->id, cmd->device->lun);
> -	DC390_UNLOCK_ACB;
> -	//return (1);
> -	done (cmd);
> -	return (0);
> +	goto fail;
>      }
>      else
>      {
> @@ -1178,52 +1154,37 @@ int DC390_queue_command (Scsi_Cmnd *cmd,
>  	 {  /* should never happen */
>  	    printk (KERN_ERR "DC390: no DCB failed, target %02x lun %02x\n",
>  		    cmd->device->id, cmd->device->lun);
> -	    DC390_UNLOCK_ACB;
> -	    printk ("DC390: No DCB in queuecommand (2)!\n");
> -#ifdef USE_NEW_EH
> -	    return (1);
> -#else
> -	    done (cmd);
> -	    return (0);
> -#endif
> +	    goto fail;
>  	 }
>      }
>
>      pACB->Cmds++;
>      cmd->scsi_done = done;
>      cmd->result = 0;
> -    if (pDCB->pWaitingSRB)
> -    {
> - 	pSRB = dc390_Free_get ( pACB );
> -	DEBUG0(if (!pSRB) printk ("DC390: No free SRB but Waiting\n"); else printk ("DC390: Free SRB w/ Waiting\n"));
> -	if (!pSRB)
> -	    return 1;
> -	else
> -	  {
> -	    dc390_BuildSRB (cmd, pDCB, pSRB);
> -	    dc390_Waiting_append (pDCB, pSRB);
> -	  }
> -	dc390_Waiting_process (pACB);
> -    }
> -    else
> -    {
> - 	pSRB = dc390_Free_get ( pACB );
> -	DEBUG0(if (!pSRB) printk ("DC390: No free SRB w/o Waiting\n"); else printk ("DC390: Free SRB w/o Waiting\n"));
> -	if (!pSRB)
> -	{
> -	    dc390_Waiting_process (pACB);
> -	    return 1;
> -	}
> -	else
> -	{
> -	    dc390_BuildSRB (cmd, pDCB, pSRB);
> -	    dc390_SendSRB (pACB, pSRB);
> -	}
> -    }
> +
> +    pSRB = dc390_Free_get(pACB);
> +    if (!pSRB)
> +	    goto requeue;
> +
> +    dc390_BuildSRB(cmd, pDCB, pSRB);
> +    if (pDCB->pWaitingSRB) {
> +	    dc390_Waiting_append(pDCB, pSRB);
> +	    dc390_Waiting_process(pACB);
> +    } else
> +	    dc390_SendSRB(pACB, pSRB);
>
>      DC390_UNLOCK_ACB;
>      DEBUG1(printk (KERN_DEBUG " ... command (pid %li) queued successfully.\n", cmd->pid));
>      return(0);
> +
> + requeue:
> +    DC390_UNLOCK_ACB;
> +    return 1;
> + fail:
> +    DC390_UNLOCK_ACB;
> +    cmd->result = DID_BAD_TARGET << 16;
> +    done(cmd);
> +    return 0;
>  }
>
>  /* We ignore mapping problems, as we expect everybody to respect
>
>

---
Guennadi Liakhovetski



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 2.6] tmscsim - no internal command-queuing
  2004-02-03 22:41   ` Guennadi Liakhovetski
@ 2004-02-19 15:13     ` Christoph Hellwig
  2004-05-20 23:02       ` Kurt Garloff
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2004-02-19 15:13 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Christoph Hellwig, linux-scsi, Kurt Garloff

> Ouch. Whereas the rest are either rather intelligent things I didn't know
> about, or something I didn't want / dare to modify without a burning need,
> this is definitely just a plain simple bug - I did see it was a noop, and
> I did think at some point to preserve its consistency, until I forgot
> about it:-)

Kurt, do you have any opinion on those patches?  I'd love to see them going
in for 2.6.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 2.6] tmscsim - no internal command-queuing
  2004-02-19 15:13     ` Christoph Hellwig
@ 2004-05-20 23:02       ` Kurt Garloff
  0 siblings, 0 replies; 6+ messages in thread
From: Kurt Garloff @ 2004-05-20 23:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Guennadi Liakhovetski, linux-scsi

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Thu, Feb 19, 2004 at 03:13:33PM +0000, Christoph Hellwig wrote:
> > Ouch. Whereas the rest are either rather intelligent things I didn't know
> > about, or something I didn't want / dare to modify without a burning need,
> > this is definitely just a plain simple bug - I did see it was a noop, and
> > I did think at some point to preserve its consistency, until I forgot
> > about it:-)
> 
> Kurt, do you have any opinion on those patches?  I'd love to see them going
> in for 2.6.4

Please merge them.

-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG / Novell, Nuernberg, DE               Director SUSE Labs

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-05-20 23:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-01-30 23:23 [PATCH, 2.6] tmscsim - no internal command-queuing Guennadi Liakhovetski
2004-02-01 11:17 ` Christoph Hellwig
2004-02-01 15:41   ` James Bottomley
2004-02-03 22:41   ` Guennadi Liakhovetski
2004-02-19 15:13     ` Christoph Hellwig
2004-05-20 23:02       ` Kurt Garloff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox