eata irq abuse (was: Re: Linux 2.5.60)

eata irq abuse (was: Re: Linux 2.5.60)

[Manfred Spraul]

Zephaniah wrote:

>kernel BUG at mm/slab.c:1102!

Slab notices that a function that expects enabled local interrupts is called with disabled local interrupts.


>Call Trace:
> [<c014a3b3>] do_tune_cpucache+0x83/0x240

do_tune_cpucache:
the function call smp_call_function(), and that is only permitted with enabled local interrupts. The complain is correct.


> [<c014a300>] do_ccpupdate_local+0x0/0x30
> [<c014a5c1>] enable_cpucache+0x51/0x80
> [<c0148ea5>] kmem_cache_create+0x4a5/0x560

Within kmem_cache_create. kmem_cache_create checks for in_interrupt(), thus someone probably does

	spin_lock_irqsave();
	kmem_cache_create();


> [<c0285dd2>] scsi_setup_command_freelist+0xa2/0x130

calls kmem_cache_create()

> [<c02887e0>] scsi_register+0x3c0/0x660

calls scsi_setup_command_freelist


> [<c02919a1>] get_pci_dev+0x31/0x50

?? probably stale

> [<c0291df2>] port_detect+0x3c2/0xe50

Do you have an eata scsi controller?

Ugs.
eata2x_detect():
* spin_lock_irqsave();
* calls port_detect();
* * spin_unlock();
* * scsi_register.

Eata maintainers: Is that necessary?
Why do the interrupts remain disabled across scsi_register?
Is that a bug workaround, or an oversight?
I'd use

	spin_unlock_irq();
	scsi_register();
	spin_lock_irq();

--
	Manfred

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Zephaniah E. Hull]

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

On Tue, Feb 11, 2003 at 06:45:35PM +0100, Manfred Spraul wrote:
> >[<c0291df2>] port_detect+0x3c2/0xe50
> 
> Do you have an eata scsi controller?

Yes I do, does exactly what I need most of the time.

The problem goes away if I compile SCSI as modules, however it happens
when I try to load the modules, so..

Anything I can do to help?

Zephaniah E. Hull.

-- 
	1024D/E65A7801 Zephaniah E. Hull <warp@babylon.d2dc.net>
	   92ED 94E4 B1E6 3624 226D  5727 4453 008B E65A 7801
	    CCs of replies from mailing lists are requested.

"This system operates under martial law. The constitution is suspended. You
 have no rights except as declared by the area commander. Violators will be
  shot. Repeat violators will be repeatedly shot...."       -from "A_W_O_L"

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

RE: eata irq abuse (was: Re: Linux 2.5.60)

[Ballabio_Dario]

Yes, you are correct. I used spin_unlock in order to release the local
driver lock
during the scsi_register call, but I forgot that I had the irq disabled as
well.
SO the correct fix is to use spin_unlock_irq/spin_lock_irq around the
scsi_register call. Same fix applies to the u14-34f driver.

Cheers,

*********************************
Ph.D. Dario Ballabio
EMC Computer Systems Italia spa
Mobile phone +393487978851
Office phone +390244571315
Mobile fax   +393487951622

*** Si vis pacem, para bellum ***


-----Original Message-----
From: Manfred Spraul [mailto:manfred@colorfullife.com]
Sent: Tuesday, February 11, 2003 6:46 PM
To: Zephaniah E. Hull
Cc: linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org;
linux-eata@i-connect.net
Subject: eata irq abuse (was: Re: Linux 2.5.60)


Zephaniah wrote:

>kernel BUG at mm/slab.c:1102!

Slab notices that a function that expects enabled local interrupts is called
with disabled local interrupts.


>Call Trace:
> [<c014a3b3>] do_tune_cpucache+0x83/0x240

do_tune_cpucache:
the function call smp_call_function(), and that is only permitted with
enabled local interrupts. The complain is correct.


> [<c014a300>] do_ccpupdate_local+0x0/0x30
> [<c014a5c1>] enable_cpucache+0x51/0x80
> [<c0148ea5>] kmem_cache_create+0x4a5/0x560

Within kmem_cache_create. kmem_cache_create checks for in_interrupt(), thus
someone probably does

	spin_lock_irqsave();
	kmem_cache_create();


> [<c0285dd2>] scsi_setup_command_freelist+0xa2/0x130

calls kmem_cache_create()

> [<c02887e0>] scsi_register+0x3c0/0x660

calls scsi_setup_command_freelist


> [<c02919a1>] get_pci_dev+0x31/0x50

?? probably stale

> [<c0291df2>] port_detect+0x3c2/0xe50

Do you have an eata scsi controller?

Ugs.
eata2x_detect():
* spin_lock_irqsave();
* calls port_detect();
* * spin_unlock();
* * scsi_register.

Eata maintainers: Is that necessary?
Why do the interrupts remain disabled across scsi_register?
Is that a bug workaround, or an oversight?
I'd use

	spin_unlock_irq();
	scsi_register();
	spin_lock_irq();

--
	Manfred


-
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: eata irq abuse (was: Re: Linux 2.5.60)

[Ballabio_Dario]

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

The enclosed patch applies to 2.5.60 and fixes the irq issue when calling
scsi_register for both eata and u14-34f drivers.
Cheers,
-db



-----Original Message-----
From: Manfred Spraul [mailto:manfred@colorfullife.com]
Sent: Tuesday, February 11, 2003 6:46 PM
To: Zephaniah E. Hull
Cc: linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org;
linux-eata@i-connect.net
Subject: eata irq abuse (was: Re: Linux 2.5.60)


Zephaniah wrote:

>kernel BUG at mm/slab.c:1102!

Slab notices that a function that expects enabled local interrupts is called
with disabled local interrupts.


>Call Trace:
> [<c014a3b3>] do_tune_cpucache+0x83/0x240

do_tune_cpucache:
the function call smp_call_function(), and that is only permitted with
enabled local interrupts. The complain is correct.


> [<c014a300>] do_ccpupdate_local+0x0/0x30
> [<c014a5c1>] enable_cpucache+0x51/0x80
> [<c0148ea5>] kmem_cache_create+0x4a5/0x560

Within kmem_cache_create. kmem_cache_create checks for in_interrupt(), thus
someone probably does

	spin_lock_irqsave();
	kmem_cache_create();


> [<c0285dd2>] scsi_setup_command_freelist+0xa2/0x130

calls kmem_cache_create()

> [<c02887e0>] scsi_register+0x3c0/0x660

calls scsi_setup_command_freelist


> [<c02919a1>] get_pci_dev+0x31/0x50

?? probably stale

> [<c0291df2>] port_detect+0x3c2/0xe50

Do you have an eata scsi controller?

Ugs.
eata2x_detect():
* spin_lock_irqsave();
* calls port_detect();
* * spin_unlock();
* * scsi_register.

Eata maintainers: Is that necessary?
Why do the interrupts remain disabled across scsi_register?
Is that a bug workaround, or an oversight?
I'd use

	spin_unlock_irq();
	scsi_register();
	spin_lock_irq();

--
	Manfred


-
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


[-- Attachment #2: drivers_804.diff --]
[-- Type: application/octet-stream, Size: 4508 bytes --]

diff -u -r linux.803/drivers/scsi/eata.c linux.804/drivers/scsi/eata.c
--- linux.803/drivers/scsi/eata.c	Wed Feb 12 12:14:02 2003
+++ linux.804/drivers/scsi/eata.c	Wed Feb 12 11:17:25 2003
@@ -1,6 +1,9 @@
 /*
  *      eata.c - Low-level driver for EATA/DMA SCSI host adapters.
  *
+ *      12 Feb 2003 Rev. 8.04 for linux 2.5.60
+ *        + Release irq before calling scsi_register.
+ *
  *      12 Nov 2002 Rev. 8.02 for linux 2.5.47
  *        + Release driver_lock before calling scsi_register.
  *
@@ -279,7 +282,7 @@
  *          This driver is based on the CAM (Common Access Method Committee)
  *          EATA (Enhanced AT Bus Attachment) rev. 2.0A, using DMA protocol.
  *
- *  Copyright (C) 1994-2002 Dario Ballabio (ballabio_dario@emc.com)
+ *  Copyright (C) 1994-2003 Dario Ballabio (ballabio_dario@emc.com)
  *
  *  Alternate email: dario.ballabio@inwind.it, dario.ballabio@tiscalinet.it
  *
@@ -1193,9 +1196,9 @@
    }
 #endif
 
-   spin_unlock(&driver_lock);
+   spin_unlock_irq(&driver_lock);
    sh[j] = scsi_register(tpnt, sizeof(struct hostdata));
-   spin_lock(&driver_lock);
+   spin_lock_irq(&driver_lock);
 
    if (sh[j] == NULL) {
       printk("%s: unable to register host, detaching.\n", name);
@@ -1306,7 +1309,7 @@
       tag_mode = TAG_ORDERED;
 
    if (j == 0) {
-      printk("EATA/DMA 2.0x: Copyright (C) 1994-2002 Dario Ballabio.\n");
+      printk("EATA/DMA 2.0x: Copyright (C) 1994-2003 Dario Ballabio.\n");
       printk("%s config options -> tm:%d, lc:%c, mq:%d, rs:%c, et:%c, "\
              "ip:%c, ep:%c, pp:%c.\n", driver_name, tag_mode,
              YESNO(linked_comm), max_queue_depth, YESNO(rev_scan),
diff -u -r linux.803/drivers/scsi/eata.h linux.804/drivers/scsi/eata.h
--- linux.803/drivers/scsi/eata.h	Wed Feb 12 12:14:12 2003
+++ linux.804/drivers/scsi/eata.h	Wed Feb 12 11:17:20 2003
@@ -11,7 +11,7 @@
                              sector_t, int *);
 static int eata2x_slave_configure(Scsi_Device *);
 
-#define EATA_VERSION "8.03.00"
+#define EATA_VERSION "8.04.00"
 
 #define EATA {                                                               \
                 .name              = "EATA/DMA 2.0x rev. " EATA_VERSION " ",   \
diff -u -r linux.803/drivers/scsi/u14-34f.c linux.804/drivers/scsi/u14-34f.c
--- linux.803/drivers/scsi/u14-34f.c	Wed Feb 12 12:14:56 2003
+++ linux.804/drivers/scsi/u14-34f.c	Wed Feb 12 11:17:46 2003
@@ -1,6 +1,9 @@
 /*
  *      u14-34f.c - Low-level driver for UltraStor 14F/34F SCSI host adapters.
  *
+ *      12 Feb 2003 Rev. 8.04 for linux 2.5.60
+ *        + Release irq before calling scsi_register.
+ *
  *      12 Nov 2002 Rev. 8.02 for linux 2.5.47
  *        + Release driver_lock before calling scsi_register.
  *
@@ -221,7 +224,7 @@
  *
  *          Multiple U14F and/or U34F host adapters are supported.
  *
- *  Copyright (C) 1994-2002 Dario Ballabio (ballabio_dario@emc.com)
+ *  Copyright (C) 1994-2003 Dario Ballabio (ballabio_dario@emc.com)
  *
  *  Alternate email: dario.ballabio@inwind.it, dario.ballabio@tiscalinet.it
  *
@@ -847,9 +850,9 @@
 
    if (have_old_firmware) tpnt->use_clustering = DISABLE_CLUSTERING;
 
-   spin_unlock(&driver_lock);
+   spin_unlock_irq(&driver_lock);
    sh[j] = scsi_register(tpnt, sizeof(struct hostdata));
-   spin_lock(&driver_lock);
+   spin_lock_irq(&driver_lock);
 
    if (sh[j] == NULL) {
       printk("%s: unable to register host, detaching.\n", name);
@@ -961,7 +964,7 @@
       tag_mode = TAG_ORDERED;
 
    if (j == 0) {
-      printk("UltraStor 14F/34F: Copyright (C) 1994-2002 Dario Ballabio.\n");
+      printk("UltraStor 14F/34F: Copyright (C) 1994-2003 Dario Ballabio.\n");
       printk("%s config options -> of:%c, tm:%d, lc:%c, mq:%d, et:%c.\n",
              driver_name, YESNO(have_old_firmware), tag_mode,
              YESNO(linked_comm), max_queue_depth, YESNO(ext_tran));
diff -u -r linux.803/drivers/scsi/u14-34f.h linux.804/drivers/scsi/u14-34f.h
--- linux.803/drivers/scsi/u14-34f.h	Wed Feb 12 12:15:06 2003
+++ linux.804/drivers/scsi/u14-34f.h	Wed Feb 12 11:17:54 2003
@@ -11,7 +11,7 @@
                               sector_t, int *);
 static int u14_34f_slave_configure(Scsi_Device *);
 
-#define U14_34F_VERSION "8.03.00"
+#define U14_34F_VERSION "8.04.00"
 
 #define ULTRASTOR_14_34F {                                                   \
                 .name         = "UltraStor 14F/34F rev. " U14_34F_VERSION " ", \

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Christoph Hellwig]

On Wed, Feb 12, 2003 at 07:47:01AM -0500, Ballabio_Dario@emc.com wrote:
> The enclosed patch applies to 2.5.60 and fixes the irq issue when calling
> scsi_register for both eata and u14-34f drivers.

Any chance you could explain us

(1) what does this lock protect
(2) why does it need to disable interrupts

RE: eata irq abuse (was: Re: Linux 2.5.60)

[Ballabio_Dario]

Last time I tried to run the detect routine with interrupts enabled,
it just hung at the first occurrence on an inb() from
the board registers. If someone is able to have it 
working with interrupts enabled and qualifies the
solution on all the platforms and eata boards,
I'd be more than happy to remove it.

-db


-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org]
Sent: Wednesday, February 12, 2003 2:17 PM
To: Ballabio_Dario@emc.com
Cc: manfred@colorfullife.com; warp@mercury.d2dc.net;
linux-scsi@vger.kernel.org
Subject: Re: eata irq abuse (was: Re: Linux 2.5.60)


On Wed, Feb 12, 2003 at 07:47:01AM -0500, Ballabio_Dario@emc.com wrote:
> The enclosed patch applies to 2.5.60 and fixes the irq issue when calling
> scsi_register for both eata and u14-34f drivers.

Any chance you could explain us

(1) what does this lock protect
(2) why does it need to disable interrupts

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Christoph Hellwig]

On Wed, Feb 12, 2003 at 09:13:10AM -0500, Ballabio_Dario@emc.com wrote:
> Last time I tried to run the detect routine with interrupts enabled,
> it just hung at the first occurrence on an inb() from
> the board registers. If someone is able to have it 
> working with interrupts enabled and qualifies the
> solution on all the platforms and eata boards,
> I'd be more than happy to remove it.

Ah, you don't need the lock but the disabled interrupts!

What about adding

local_irq_disable();
...
local_irq_enable();

around the sections that need interreupts disabled?  The spinlock is
really confusing.

Re: eata irq abuse (was: Re: Linux 2.5.60)

[James Bottomley]

On Wed, 2003-02-12 at 08:25, Christoph Hellwig wrote:
> On Wed, Feb 12, 2003 at 09:13:10AM -0500, Ballabio_Dario@emc.com wrote:
> > Last time I tried to run the detect routine with interrupts enabled,
> > it just hung at the first occurrence on an inb() from
> > the board registers. If someone is able to have it 
> > working with interrupts enabled and qualifies the
> > solution on all the platforms and eata boards,
> > I'd be more than happy to remove it.
> 
> Ah, you don't need the lock but the disabled interrupts!

In general, that's not correct: most HBA registers have to be accessed
in sequence, thus you need global protection from anyone else touching
them while you're at the registers.  This is what host_lock was designed
for.

James

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Christoph Hellwig]

On Wed, Feb 12, 2003 at 08:40:34AM -0600, James Bottomley wrote:
> > Ah, you don't need the lock but the disabled interrupts!
> 
> In general, that's not correct: most HBA registers have to be accessed
> in sequence, thus you need global protection from anyone else touching
> them while you're at the registers.  This is what host_lock was designed
> for.

Of course.  but that's not how driver_lock in eata.c works.  It's taken
when we enter eata2x_detect(), dropped before scsi_register(), reacquired
afterwards and released when we leave eata2x_detect().  I don't really
see what it is supposed to protect.

Re: eata irq abuse (was: Re: Linux 2.5.60)

[James Bottomley]

On Wed, 2003-02-12 at 08:51, Christoph Hellwig wrote:
> On Wed, Feb 12, 2003 at 08:40:34AM -0600, James Bottomley wrote:
> > > Ah, you don't need the lock but the disabled interrupts!
> > 
> > In general, that's not correct: most HBA registers have to be accessed
> > in sequence, thus you need global protection from anyone else touching
> > them while you're at the registers.  This is what host_lock was designed
> > for.
> 
> Of course.  but that's not how driver_lock in eata.c works.  It's taken
> when we enter eata2x_detect(), dropped before scsi_register(), reacquired
> afterwards and released when we leave eata2x_detect().  I don't really
> see what it is supposed to protect.

I agree, I'll look over the driver while I'm travelling today and see if
we can do better.  I just didn't want to give the impression that
local_irq_disable() is sufficient protection when accessing board
registers (having seen one or two SMP problems related to this).

James

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Jeff Garzik]

Ballabio_Dario@emc.com wrote:
> Yes, you are correct. I used spin_unlock in order to release the local
> driver lock
> during the scsi_register call, but I forgot that I had the irq disabled as
> well.
> SO the correct fix is to use spin_unlock_irq/spin_lock_irq around the
> scsi_register call. Same fix applies to the u14-34f driver.


scsi_register may want to sleep, so that is not a fix at all...

RE: eata irq abuse (was: Re: Linux 2.5.60)

[Ballabio_Dario]

local_irq_disable/enable could be sufficient as long as
there are not two instances of the detect routine 
running simultaneously, which is hopefully the case
with actual lk, even when loading the driver as a module.
Moreover the local_irq primitives are relatively new,
while spin_lock_irq has been available for years. 
There is nothing special with eata driver concerning
this aspect, what you see is common to the majority
of scsi low level drivers. As long as interrupts are
disabled while accessing board registers _and_ the
detect routine is known to lk to be non re-entrant, any
protection scheme would fit.
-db


-----Original Message-----
From: James Bottomley [mailto:James.Bottomley@steeleye.com]
Sent: Wednesday, February 12, 2003 3:55 PM
To: Christoph Hellwig
Cc: Ballabio_Dario@emc.com; manfred@colorfullife.com;
warp@mercury.d2dc.net; SCSI Mailing List
Subject: Re: eata irq abuse (was: Re: Linux 2.5.60)


On Wed, 2003-02-12 at 08:51, Christoph Hellwig wrote:
> On Wed, Feb 12, 2003 at 08:40:34AM -0600, James Bottomley wrote:
> > > Ah, you don't need the lock but the disabled interrupts!
> > 
> > In general, that's not correct: most HBA registers have to be accessed
> > in sequence, thus you need global protection from anyone else touching
> > them while you're at the registers.  This is what host_lock was designed
> > for.
> 
> Of course.  but that's not how driver_lock in eata.c works.  It's taken
> when we enter eata2x_detect(), dropped before scsi_register(), reacquired
> afterwards and released when we leave eata2x_detect().  I don't really
> see what it is supposed to protect.

I agree, I'll look over the driver while I'm travelling today and see if
we can do better.  I just didn't want to give the impression that
local_irq_disable() is sufficient protection when accessing board
registers (having seen one or two SMP problems related to this).

James

Re: eata irq abuse (was: Re: Linux 2.5.60)

[Jeff Garzik]

Jeff Garzik wrote:
> Ballabio_Dario@emc.com wrote:
> 
>> Yes, you are correct. I used spin_unlock in order to release the local
>> driver lock
>> during the scsi_register call, but I forgot that I had the irq 
>> disabled as
>> well.
>> SO the correct fix is to use spin_unlock_irq/spin_lock_irq around the
>> scsi_register call. Same fix applies to the u14-34f driver.
> 
> scsi_register may want to sleep, so that is not a fix at all...


Ooops, I missed the order.  You (and Manfred) are right, 
unlock-register-lock is desired.

ENOCAFFEINE, I plead...

RE: eata irq abuse (was: Re: Linux 2.5.60)

[Suresh Kr N]

hi everyone

I am working on a san management application for a linux. My application
sends zoning commands to the fabric to zone in a disk to a host ,  so i need
to have that disk acessible to the host without a reboot of the host.

I need to do this from the user space. Is there a way to ask the scsi
midlayer for a rescan ?
In solaris , i invoke a command called "drvconfig" to achieve this.

I have seen in the linux-scsi archive that this is kind of possible,  but
the discussion didnt seem to state how.
I am attaching the above thread here in the mail.

I have verified that echo "scsi add-single-device a b c d" > /proc/scsi/scsi
works fine . But this does involve parameters which cant be found out
programmatically.

Also a side question here :In a FC world, to get the target number for above
command , can i find out the current maximum target number of the scsi
devices, and increase it by one ? or  does the FC devices have actually a
preknown target no , which would be collected by the FC card driver ?

Could someone please help ? ( even a "non-standard" way should be good
enough" )

Thanks
Suresh



--------------------------------------------------------
Message 2 in thread
From: Alan Cox (alan@lxorguk.ukuu.org.uk)
Subject: Re: Hot plugging a disk in runtime
View this article only
Newsgroups: mlist.linux.scsi
Date: 2002-05-20 05:32:38 PST


> 1.. How is it possible to signal SCSI mid-layer (or other layers) of a
> presence of a newly hot-plugged disk ?

aacraid is one example that does this, but via its config tools.

> 2.. Can an adapter's device driver trigger SCSI subsystem to scan
> channels for new LUNs / Targets ?

It is designed so user space can do so - so yes

> 3.. Is this can be handled in a kernel level ? is there a user space
> application that can be triggered to start scanning adapters (obviously
> through SCSI subsystem) and registering new LUNs / Targets etc... ?

echo "scsi-add-single-device a b c d" /proc/scsi/scsi

if I remember it right


entire thread is at

http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&threadm=linux.scsi.200205
201420.g4KEK3x01892%40localhost.localdomain&rnum=7&prev=/groups%3Fq%3Dhot%2B
plug%2B%2BSCSI%2Bfibre-channel%2BLinux%26hl%3Den%26lr%3D%26ie%3DUTF-8%26selm
%3Dlinux.scsi.200205201420.g4KEK3x01892%2540localhost.localdomain%26rnum%3D7