Assumption in sym53c8xx.c failed

DISCUSSION Christian Kurz 2 participants 3 messages Mar 31, 2001 – Apr 3, 2001

Share & tools

Christian Kurz

author

2001-03-31 22:01 UTC #0 lore raw

Hi,

I'm currently running 2.4.2-ac28 and today I got a failing assumption in sym53c8xx.c. I'm not sure about the exact steps that I did to produce this error, but it must have been something like: cdparanoia -blank=all, then sending Ctrl+C to this process and after it's been killed cdparanoia -blank=fast. I then got assertion: k!=-1 failed. But I found no hint about this in the messages or syslog file. So I looked through sym53c8xx.c to find this code and it seems like line 10123 is responsible for creating this error and kernel panic. Should this be the normal behaviour or is this a bug in the code?

Christian

Signature · 2 lines

Truth is the most valuable thing we have -- so let us economize it.
                -- Mark Twain

Gérard Roudier

2001-04-02 17:29 UTC #1 lore raw

Quoted 11 lines · Christian Kurz

Hi,
I'm currently running 2.4.2-ac28 and today I got a failing assumption in sym53c8xx.c. I'm not sure about the exact steps that I did to produce this error, but it must have been something like: cdparanoia -blank=all, then sending Ctrl+C to this process and after it's been killed cdparanoia -blank=fast. I then got assertion: k!=-1 failed. But I found no hint about this in the messages or syslog file. So I looked through sym53c8xx.c to find this code and it seems like line 10123 is responsible for creating this error and kernel panic. Should this be the normal behaviour or is this a bug in the code?

This might well be both at the same time. I mean normal behaviour given a bug in the code. :-)

Could you try this tiny patch and let me know:

sym53c8xx.c renamed from sym53c8xx.c.0402 +3 -4

@@ -10167,14 +10167,13 @@
 				if (i >= MAX_START*2)
 					i = 0;
 			}
-			assert(k != -1);
 			if (k != 1) {
 				np->squeue[k] = np->squeue[i]; /* Idle task */
 				np->squeueput = k; /* Start queue pointer */
-				cp->host_status = HS_ABORTED;
-				cp->scsi_status = S_ILLEGAL;
-				ncr_complete(np, cp);
 			}
+			cp->host_status = HS_ABORTED;
+			cp->scsi_status = S_ILLEGAL;
+			ncr_complete(np, cp);
 		}
 		break;
 	/*

What happens is that this part of the driver code assumed that the CCB for an IO to abort is queued to the SCSI SCRIPTS. This is not always true since the driver may temporarily not queue all IOs to SCRIPTS. This may happens on QUEUE FULL condition or for devices that donnot accept tagged commands, for example.

Regards,
Gérard.

Gérard Roudier

2001-04-03 08:35 UTC #2 lore raw

Hello,

The patch I sent you yesterday has a typo that can lead to serious breakage. This has been pointed out by Michal Jaegermann. Thanks, Michal. (k must be compared against -1 and not against 1)

You want to apply this new tiny patch instead and let me know if it fixes:

sym53c8xx.c renamed from sym53c8xx.c.0402 +4 -5

@@ -10167,14 +10167,13 @@
 				if (i >= MAX_START*2)
 					i = 0;
 			}
-			assert(k != -1);
-			if (k != 1) {
+			if (k != -1) {
 				np->squeue[k] = np->squeue[i]; /* Idle task */
 				np->squeueput = k; /* Start queue pointer */
-				cp->host_status = HS_ABORTED;
-				cp->scsi_status = S_ILLEGAL;
-				ncr_complete(np, cp);
 			}
+			cp->host_status = HS_ABORTED;
+			cp->scsi_status = S_ILLEGAL;
+			ncr_complete(np, cp);
 		}
 		break;
 	/*

--------------------- Cut Here -----------------

Gérard.

Quoted 52 lines · Gérard Roudier

Quoted 11 lines · Christian Kurz
Hi,
I'm currently running 2.4.2-ac28 and today I got a failing assumption in sym53c8xx.c. I'm not sure about the exact steps that I did to produce this error, but it must have been something like: cdparanoia -blank=all, then sending Ctrl+C to this process and after it's been killed cdparanoia -blank=fast. I then got assertion: k!=-1 failed. But I found no hint about this in the messages or syslog file. So I looked through sym53c8xx.c to find this code and it seems like line 10123 is responsible for creating this error and kernel panic. Should this be the normal behaviour or is this a bug in the code?
This might well be both at the same time. I mean normal behaviour given a bug in the code. :-)
Could you try this tiny patch and let me know:
sym53c8xx.c renamed from sym53c8xx.c.0402 +3 -4
@@ -10167,14 +10167,13 @@
 				if (i >= MAX_START*2)
 					i = 0;
 			}
-			assert(k != -1);
 			if (k != 1) {
 				np->squeue[k] = np->squeue[i]; /* Idle task */
 				np->squeueput = k; /* Start queue pointer */
-				cp->host_status = HS_ABORTED;
-				cp->scsi_status = S_ILLEGAL;
-				ncr_complete(np, cp);
 			}
+			cp->host_status = HS_ABORTED;
+			cp->scsi_status = S_ILLEGAL;
+			ncr_complete(np, cp);
 		}
 		break;
 	/*
What happens is that this part of the driver code assumed that the CCB for an IO to abort is queued to the SCSI SCRIPTS. This is not always true since the driver may temporarily not queue all IOs to SCRIPTS. This may happens on QUEUE FULL condition or for devices that donnot accept tagged commands, for example.
Regards,
Gérard.