[PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Hannes Reinecke]

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

This patch converts platform_data->qfrozen to atomic_t.
This way we can get rid of ahd_lock / ahd_unlock for the
accessor functions; it also fixes some deadlocks in the
recovery code (again).

Signed-off-by: Hannes Reinecke <hare@suse.de>

-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

[-- Attachment #2: 0001-aic79xx-convert-qfrozen-to-atomic_t.txt --]
[-- Type: text/plain, Size: 4195 bytes --]

Subject: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

This patch converts platform_data->qfrozen to atomic_t.
This way we can get rid of ahd_lock / ahd_unlock for the
accessor functions; it also fixes some deadlocks in the
recovery code (again).

Signed-off-by: Hannes Reinecke <hare@suse.de>

---

 drivers/scsi/aic7xxx/aic79xx_osm.c |   42 +++++++++---------------------------
 drivers/scsi/aic7xxx/aic79xx_osm.h |    2 +-
 2 files changed, 11 insertions(+), 33 deletions(-)

872e277a460c57d74bfb478fdbc86aae02f37198
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 78c8338..bd2be6a 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -453,18 +453,15 @@ ahd_linux_queue(struct scsi_cmnd * cmd, 
 	struct	 ahd_softc *ahd;
 	struct	 ahd_linux_device *dev = scsi_transport_device_data(cmd->device);
 	int rtn = SCSI_MLQUEUE_HOST_BUSY;
-	unsigned long flags;
 
 	ahd = *(struct ahd_softc **)cmd->device->host->hostdata;
 
-	ahd_lock(ahd, &flags);
-	if (ahd->platform_data->qfrozen == 0) {
+	if (!atomic_read(&ahd->platform_data->qfrozen)) {
 		cmd->scsi_done = scsi_done;
 		cmd->result = CAM_REQ_INPROG << 16;
 		rtn = ahd_linux_run_command(ahd, dev, cmd);
 
 	}
-	ahd_unlock(ahd, &flags);
 	return rtn;
 }
 
@@ -691,10 +688,8 @@ ahd_linux_bus_reset(struct scsi_cmnd *cm
 		printf("%s: Bus reset called for cmd %p\n",
 		       ahd_name(ahd), cmd);
 #endif
-	ahd_lock(ahd, &s);
 	found = ahd_reset_channel(ahd, scmd_channel(cmd) + 'A',
 				  /*initiate reset*/TRUE);
-	ahd_unlock(ahd, &s);
 
 	if (bootverbose)
 		printf("%s: SCSI bus reset delivered. "
@@ -1194,6 +1189,7 @@ ahd_platform_alloc(struct ahd_softc *ahd
 	memset(ahd->platform_data, 0, sizeof(struct ahd_platform_data));
 	ahd->platform_data->irq = AHD_LINUX_NOIRQ;
 	ahd_lockinit(ahd);
+	atomic_set(&ahd->platform_data->qfrozen, 0);
 	init_MUTEX_LOCKED(&ahd->platform_data->eh_sem);
 	ahd->seltime = (aic79xx_seltime & 0x3) << 4;
 	return (0);
@@ -1443,6 +1439,9 @@ ahd_linux_run_command(struct ahd_softc *
 	struct	 ahd_tmode_tstate *tstate;
 	u_int	 col_idx;
 	uint16_t mask;
+	unsigned long flags;
+
+	ahd_lock(ahd, &flags);
 
 	/*
 	 * Get an scb to use.
@@ -1458,6 +1457,7 @@ ahd_linux_run_command(struct ahd_softc *
 	}
 	if ((scb = ahd_get_scb(ahd, col_idx)) == NULL) {
 		ahd->flags |= AHD_RESOURCE_SHORTAGE;
+		ahd_unlock(ahd, &flags);
 		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
@@ -1583,6 +1583,8 @@ ahd_linux_run_command(struct ahd_softc *
 	scb->flags |= SCB_ACTIVE;
 	ahd_queue_scb(ahd, scb);
 
+	ahd_unlock(ahd, &flags);
+
 	return 0;
 }
 
@@ -2051,38 +2053,14 @@ ahd_freeze_simq(struct ahd_softc *ahd)
 {
 	unsigned long s;
 
-	ahd_lock(ahd, &s);
-	ahd->platform_data->qfrozen++;
-	if (ahd->platform_data->qfrozen == 1) {
+	if (!atomic_inc_and_test(&ahd->platform_data->qfrozen))
 		scsi_block_requests(ahd->platform_data->host);
-		ahd_platform_abort_scbs(ahd, CAM_TARGET_WILDCARD, ALL_CHANNELS,
-					CAM_LUN_WILDCARD, SCB_LIST_NULL,
-					ROLE_INITIATOR, CAM_REQUEUE_REQ);
-	}
-	ahd_unlock(ahd, &s);
 }
 
 void
 ahd_release_simq(struct ahd_softc *ahd)
 {
-	u_long s;
-	int    unblock_reqs;
-
-	unblock_reqs = 0;
-	ahd_lock(ahd, &s);
-	if (ahd->platform_data->qfrozen > 0)
-		ahd->platform_data->qfrozen--;
-	if (ahd->platform_data->qfrozen == 0) {
-		unblock_reqs = 1;
-	}
-	ahd_unlock(ahd, &s);
-	/*
-	 * There is still a race here.  The mid-layer
-	 * should keep its own freeze count and use
-	 * a bottom half handler to run the queues
-	 * so we can unblock with our own lock held.
-	 */
-	if (unblock_reqs)
+	if (atomic_dec_and_test(&ahd->platform_data->qfrozen))
 		scsi_unblock_requests(ahd->platform_data->host);
 }
 
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.h b/drivers/scsi/aic7xxx/aic79xx_osm.h
index 9cb1013..1f60590 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.h
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.h
@@ -381,7 +381,7 @@ struct ahd_platform_data {
 	struct scsi_target *starget[AHD_NUM_TARGETS]; 
 
 	spinlock_t		 spin_lock;
-	u_int			 qfrozen;
+	atomic_t		 qfrozen;
 	struct semaphore	 eh_sem;
 	struct Scsi_Host        *host;		/* pointer to scsi host */
 #define AHD_LINUX_NOIRQ	((uint32_t)~0)
-- 
1.1.3

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Christoph Hellwig]

On Tue, Feb 07, 2006 at 08:52:50AM +0100, Hannes Reinecke wrote:
> This patch converts platform_data->qfrozen to atomic_t.
> This way we can get rid of ahd_lock / ahd_unlock for the
> accessor functions; it also fixes some deadlocks in the
> recovery code (again).

Is ahd_freeze_simq called recursively at all?  If not it would be much
better to just kill it completely and just rely on the block layer, as
we do in all the other drivers.

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Hannes Reinecke]

Christoph Hellwig wrote:
> On Tue, Feb 07, 2006 at 08:52:50AM +0100, Hannes Reinecke wrote:
>> This patch converts platform_data->qfrozen to atomic_t.
>> This way we can get rid of ahd_lock / ahd_unlock for the
>> accessor functions; it also fixes some deadlocks in the
>> recovery code (again).
> 
> Is ahd_freeze_simq called recursively at all?  If not it would be much
> better to just kill it completely and just rely on the block layer, as
> we do in all the other drivers.
> 

As in

void
ahd_freeze_simq(struct ahd_softc *ahd)
{
	scsi_block_requests(ahd->platform_data->host);
}

?

Hmm. Looks as if this could work ...

Lemme check.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Christoph Hellwig]

On Tue, Feb 07, 2006 at 08:52:50AM +0100, Hannes Reinecke wrote:
> This patch converts platform_data->qfrozen to atomic_t.
> This way we can get rid of ahd_lock / ahd_unlock for the
> accessor functions; it also fixes some deadlocks in the
> recovery code (again).

While we're at it there's also a qfrozen variable in the ahd_linux_device
structure.  Since the driver lost it's internal queueing it and the
surrounding core are totally unused.


Index: linux-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.c	2006-01-31 12:23:38.000000000 +0100
+++ linux-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c	2006-02-07 19:52:47.000000000 +0100
@@ -1290,12 +1290,6 @@
 		now_queuing = AHD_DEV_Q_TAGGED;
 		break;
 	}
-	if ((dev->flags & AHD_DEV_FREEZE_TIL_EMPTY) == 0
-	 && (was_queuing != now_queuing)
-	 && (dev->active != 0)) {
-		dev->flags |= AHD_DEV_FREEZE_TIL_EMPTY;
-		dev->qfrozen++;
-	}
 
 	dev->flags &= ~(AHD_DEV_Q_BASIC|AHD_DEV_Q_TAGGED|AHD_DEV_PERIODIC_OTAG);
 	if (now_queuing) {
@@ -1705,10 +1699,7 @@
 	dev = scb->platform_data->dev;
 	dev->active--;
 	dev->openings++;
-	if ((cmd->result & (CAM_DEV_QFRZN << 16)) != 0) {
-		cmd->result &= ~(CAM_DEV_QFRZN << 16);
-		dev->qfrozen--;
-	}
+
 	ahd_linux_unmap_scb(ahd, scb);
 
 	/*
Index: linux-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h
===================================================================
--- linux-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.h	2006-01-31 12:23:38.000000000 +0100
+++ linux-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h	2006-02-07 19:52:57.000000000 +0100
@@ -264,7 +264,6 @@
  */
 
 typedef enum {
-	AHD_DEV_FREEZE_TIL_EMPTY = 0x02, /* Freeze queue until active == 0 */
 	AHD_DEV_Q_BASIC		 = 0x10, /* Allow basic device queuing */
 	AHD_DEV_Q_TAGGED	 = 0x20, /* Allow full SCSI2 command queueing */
 	AHD_DEV_PERIODIC_OTAG	 = 0x40, /* Send OTAG to prevent starvation */
@@ -291,12 +290,6 @@
 	int			openings;
 
 	/*
-	 * A positive count indicates that this
-	 * device's queue is halted.
-	 */
-	u_int			qfrozen;
-	
-	/*
 	 * Cumulative command counter.
 	 */
 	u_long			commands_issued;
@@ -870,10 +863,6 @@
 static __inline void
 ahd_freeze_scb(struct scb *scb)
 {
-	if ((scb->io_ctx->result & (CAM_DEV_QFRZN << 16)) == 0) {
-                scb->io_ctx->result |= CAM_DEV_QFRZN << 16;
-                scb->platform_data->dev->qfrozen++;
-        }
 }
 
 void	ahd_platform_set_tags(struct ahd_softc *ahd,
Index: linux-2.6/drivers/scsi/aic7xxx/aic79xx_proc.c
===================================================================
--- linux-2.6.orig/drivers/scsi/aic7xxx/aic79xx_proc.c	2005-10-31 12:23:45.000000000 +0100
+++ linux-2.6/drivers/scsi/aic7xxx/aic79xx_proc.c	2006-02-07 18:12:11.000000000 +0100
@@ -255,7 +255,6 @@
 	copy_info(info, "\t\tCommands Active %d\n", dev->active);
 	copy_info(info, "\t\tCommand Openings %d\n", dev->openings);
 	copy_info(info, "\t\tMax Tagged Openings %d\n", dev->maxtags);
-	copy_info(info, "\t\tDevice Queue Frozen Count %d\n", dev->qfrozen);
 }
 
 static int

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Christoph Hellwig]

On Tue, Feb 07, 2006 at 12:30:55PM +0100, Hannes Reinecke wrote:
> As in
> 
> void
> ahd_freeze_simq(struct ahd_softc *ahd)
> {
> 	scsi_block_requests(ahd->platform_data->host);
> }
> 
> ?

Yes.

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Hannes Reinecke]

Christoph Hellwig wrote:
> On Tue, Feb 07, 2006 at 08:52:50AM +0100, Hannes Reinecke wrote:
>> This patch converts platform_data->qfrozen to atomic_t.
>> This way we can get rid of ahd_lock / ahd_unlock for the
>> accessor functions; it also fixes some deadlocks in the
>> recovery code (again).
> 
> While we're at it there's also a qfrozen variable in the ahd_linux_device
> structure.  Since the driver lost it's internal queueing it and the
> surrounding core are totally unused.
> 
Not quite sure (yet) whether we can do this.
dev->qfrozen is also used during error recovery to prevent the SCB being
reused accidentally.

And as the error recovery is totally buggered anyway I'm loath to
dead-fix it even more. I'll have to investigate that.

But the platform->qfrozen flag can indeed be removed. Just calling
scsi_block_request() is enough here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Emmanuel Fusté]

> On Tue, Feb 07, 2006 at 12:30:55PM +0100, Hannes Reinecke wrote:
> > As in
> > 
> > void
> > ahd_freeze_simq(struct ahd_softc *ahd)
> > {
> > 	scsi_block_requests(ahd->platform_data->host);
> > }
> > 
> > ?
> 
> Yes.
> 
Does it mean we could simply remove the platform_data->qfrozen
flag/counter ?


---

Accédez au courrier électronique de La Poste : www.laposte.net ; 
3615 LAPOSTENET (0,34 €/mn) ; tél : 08 92 68 13 50 (0,34€/mn)



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] aic79xx: convert qfrozen to atomic_t

[Hannes Reinecke]

Emmanuel Fusté wrote:
>> On Tue, Feb 07, 2006 at 12:30:55PM +0100, Hannes Reinecke wrote:
>>> As in
>>>
>>> void
>>> ahd_freeze_simq(struct ahd_softc *ahd)
>>> {
>>> 	scsi_block_requests(ahd->platform_data->host);
>>> }
>>>
>>> ?
>> Yes.
>>
> Does it mean we could simply remove the platform_data->qfrozen
> flag/counter ?
> 
> 
Yes. I'll send an updated patchset around.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html