All of lore.kernel.org
 help / color / mirror / Atom feed
* [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2003-07-17 23:40 Andrew Vasquez
  2003-07-18 10:48 ` Christoph Hellwig
  2003-07-18 11:23 ` Christoph Hellwig
  0 siblings, 2 replies; 21+ messages in thread
From: Andrew Vasquez @ 2003-07-17 23:40 UTC (permalink / raw)
  To: Linux-Kernel, Linux-SCSI

All,

A new version of the 8.x series driver for Linux 2.6.x kernels has
been uploaded to SourceForge:

	http://sourceforge.net/projects/linux-qla2xxx/

This beta contains a large rewrite of some fundamental structures used
throughout the driver.  This change sets the stage for some of the
more interesting and useful updates such as asynchronous mailbox
commands and a less intrusive discovery processes.

Additionally, the beta includes several performance related changes:

	o An initial merge of performance patches from Arjan van de
	  Ven.  Read the revision notes for details:
	  - Lock contention fixes.
	  - RIO support.
	o SCSI command --> IOCB formation speedup.

BTW: future driver drops *will* come out at a more spirited pace.

Changes include:

 *	- IOCB staging fixups.
 *	- Full support for 2k port logins (~2000 fabric devices).
 *	- Resync with 6.06.00b12.
 *	- Resync with Linux Kernel 2.6.0-test1.

Finally, regarding some of the more interesting (if not the) question(s)
pertaining to the development of qla2xxx:

  o Creation of a single driver module rather than three distinct
	drivers for each ISP type (21xx, 22xx, and 23xx).

  From the technical side, there aren't many compelling reasons for
  the change to not occur.  Support for 2k logins on 2300s did
  introduce a rather large, but manageable (through the compile-time
  preprocessor), interface change between the host driver and
  firmware.  The driver could of course manage this during run-time
  with some creative structure overlays, etc.  Secondly, bundling
  firmware for all ISP types can lead to a rather large binary
  module (21xx - ~64kbytes, 22xx - ~90kbytes, 23xx - ~110kbytes).
  Also, when formal support for the ISP2322 chip is added, an
  additional firmware image (again ~110kbytes) will need to be
  compiled with the driver.  The new user-space firmware-load
  interface could address the size concerns, but at the same time it
  also calls into question the larger issue of support.

  Unfortunately, it is support that ultimately becomes the
  overriding factor in maintaining the three-module build process.
  By building distinct modules (i.e. qla2300.ko to support ISP2300,
  ISP2312, and ISP2322 chips) our DVT group would focus their time and
  efforts on testing 23xx HBAs and not on regressing support with
  EOL'd products.

Until a policy change, the 8.x driver in its current form will have
the limitation of only one driver, qla2100, qla2200, or qla2300, can
be built as part of the kernel at any given time.  Please note, all
three binaries built as modules *can* be loaded without incident.  For
example:

  The following combinations will NOT work:

	- 2100 and 2200 support built as part of the kernel.
	- 2200 and 2300 support built as part of the kernel.
	- 2100 and 2300 support built as part of the kernel.

  The following combinations will work:

	- 2100, 2200 and 2300 support built as modules.
	- 2100 support build as part of the kernel, 2200 and 2300
	  support built as modules.
	- 2300 support build as part of the kernel, 2100 and 2200
	  support built as modules.
	- etc...

Regards,
Andrew Vasquez

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2003-07-18 21:15 Andrew Vasquez
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Vasquez @ 2003-07-18 21:15 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux-SCSI

Christoph,

<snip>
> >   with some creative structure overlays, etc.  Secondly, bundling
> >   firmware for all ISP types can lead to a rather large binary
> >   module (21xx - ~64kbytes, 22xx - ~90kbytes, 23xx - ~110kbytes).
> 
> Well, support for each of the subtypes can be conditional or even in
> their own submodules that share a common "library" module.
> 
> >   Unfortunately, it is support that ultimately becomes the
> >   overriding factor in maintaining the three-module build process.
> 
> Well, the current build process is horrible :)
>

I don't believe we are going to disagree on any of the technical or
maintainability aspects of having a single binary (or as you mention
selectable ISP configuration options).

> > Until a policy change, the 8.x driver in its current form will
> > have the limitation of only one driver, qla2100, qla2200, or
> > qla2300, can be built as part of the kernel at any given time.
> 
> This is not acceptable for a driver in mainline, just FYI.
> 

I've been (and currently am) pushing for some resultion on this issue.

Regards,
Andrew Vasquez

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2003-07-18 21:46 Andrew Vasquez
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Vasquez @ 2003-07-18 21:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux-SCSI

Christoph,

Thank for the feedback -- comment within...

>  - Documentation/scsi/qla2xxx.txt seems to document a 2.4ish driver
>  - Documentation/scsi/qla2xxx.release.txt looks superflous, can you
>    merge it into the (updated) qla2xxx.txt?

Updates to the documentation is on my list of todos.

>  - drivers/scsi/qla2xxx/ has duplicates of the documentation
> 

Hmm, I just checked the kernel tar-ball, there aren't any docs
there. 

>  - the (v8) comments in drivers/scsi/qla2xxx/Kconfig are superflous
>  - drivers/scsi/qla2xxx/Makefile is a mess.  If you want to compile
>    a single source file multiple times in different ways create a
>    new source file that sets the needed defines and #includes the
>    source file.  But in general this should be avoided in favour of
>    a common library module..
> 

Funny, this is what we did for the 6.x series driver.  This is part of
a larger issue we'll (QLogic) will need to address.

>  - please document which files are shared with other operating systems
>    (some look like they do, don't they?)
>

All the firmware files (more on that later), a majority of the
failover code (qla_cfg(ln), qla_fo, qlfo), most definitions in
qla_def.h, all code in qla_sup.c, mailbox command handling semantics
in qla_mbx.c.  Short answer, we're still in the process of slicing and
splicing.

>  - the driver still has multipath support in the lowlevel driver which
>    we don't want in Linux


>  - UNIQUE_FW_NAME is still there.  shouldn't this be enabled 
> uncdontionally?
>

The firmware image is automatically built and distributed for all OS
platforms.  The only thing done specifically for the Linux driver is
the addition of the 'GPL' preamble to each firmware file.  I'd rather
not spin cycles cutting out #if's after each firmware drop.

>  - still tons of typedefs that want cleaning up
>

We're phasing-in the changes during each drop.  b5 will continue to
cut the cruft.

>  - your include structures is very confusing.  Please include 
> the linux
>    headers directly i nthe files using them (except of course for
>    source files shares with other OSes)
>  

We'll look into this.

>  - qla2x00_intr_handler should use spin_lock, not spin_lock_irqsave

Are you sure about that?  I'll need to refresh my interrupt handling
know-how...

>  - you're using down_interruptible without checking the return value

Noted..

>  - please switch to newstyle module/boot parameter handling 
> (module_param)
>    (oh, okay, found the TODO comment)

Will do.

>  - what is #if defined (CONFIG_SCSIFCHOTSWAP) || 
> defined(CONFIG_GAMAP)?
>    that's never defined in a 2.5 tree and the code looks ugly as hell.

Stuff that's already removed in my latest tree...

>  - .unchecked_isa_dma      = 0 in the host template is superflous,
>    it's 0 by default.

Done.

>  - what is CONFIG_MD_MULTIHOST_FC?

Removed.

>  - you are using a static buffer in qla2x00_info, there's a 
> lock needed
>    to protect it
>  - in qla2x00_queuecommand you need to call
>   spin_unlock_irq/spin_lock_irq on the hostlock, otherwise your whole
>  ->queuecommand runs with (local) interrupts disabled

Noted..

>  - qla2x00_biosparam is superflous, if there's no ->biosparam 
> the midlayer
>    calls scsicam_bios_param directly
>  - you don't need to call scsi_set_device if you pass the 
> proper struct device
>    to scsi_add_host

Thanks for the tip.

>  - please don't use scsi_assign_lock.  In 2.5 we already have 
> a per-HBA
>    lock and using a different one than the default one 
> doesn't make sense.

Yes, that patch shouldn't have gone into 8.x.

>  - you need to check the return value from scsi_add_host

Noted..

>  - please remove the "sanity check" in qla2x00_remove_device.  If you
>    can't trust the pci layer you have worse problems..
>  - ->proc_info is deprecated, please implement shost and sdev sysfs
>    attributes instead.
>  

Will do.

>  - all contents of qla_ip.c is surrounded by a big if 
> defined(FC_IP_SUPPORT),
>    please compile the file conditionally instead.  (also in 
> linux we prefer
>    #ifdef over #if defined)
>  - where is qla2200_ip_inquiry/qla2300_ip_inquiry called??  
> also it makes
>    more sense to pass the scsi_qla_host_t directrly to it 
> instead of an
>    adapter number that requires list walking
> 

With the latest fcdev/fcinitiator/ip_dev --> fcport consolidation
work, the IP code needs to be overhauled -- thus the #if 0.

Wow, thanks for the feedback!

Regards,
Andrew Vasquez
'...only 20 more 'announce' replies to go through...some friday...'

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2003-07-18 21:53 Andrew Vasquez
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Vasquez @ 2003-07-18 21:53 UTC (permalink / raw)
  To: Lars Marowsky-Bree; +Cc: Linux-SCSI, Christoph Hellwig

Lars,

> Please come and attend the multipath session at the Kernel Summit
> and/or the Ottawa Linux Symposium next week to learn why this is a
> bad idea and how it can be done better ->
> http://www.linuxsymposium.org/
> 

I would have really liked to be in Ottawa for the kernel-summit.
Unfortunately, the earliest I can arrive is on Tuesday night.  BTW:
Duane Grigsby (from QLogic), will also be attending.  I look forward
to the event.

> However, for backwards compatibility with 2.4, I assume it might
> still be needed in 2.5/2.6, but _only_ with the clear intend of
> phasing it out within 2.6 and dropping it in 2.7.
> 
> QLogic is reasonably good already in that it can be disabled via a
> module parameter (ql2xfailover=0), allowing the higher level
> solutions to operate.
> 

I believe that will have to be the best we can offer, for those that
do not want failover functionality, just disable it.

Regards,
Andrew Vasquez

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2003-07-19  1:05 Manfred Spraul
  0 siblings, 0 replies; 21+ messages in thread
From: Manfred Spraul @ 2003-07-19  1:05 UTC (permalink / raw)
  To: Andrew Vasquez
  Cc: linux-scsi, Linux Kernel Mailing List, Christoph Hellwig,
	Arjan van de Ven

Andrew wrote:

>>  - qla2x00_intr_handler should use spin_lock, not spin_lock_irqsave
>
>Are you sure about that?  I'll need to refresh my interrupt handling
>know-how...
>  
>
It's an optimization that is used by many drivers:
Interrupt handlers are never reentered - if you are within 
qla2x00_intr_handler handling irq x, then it's guaranteed that the 
function won't be reentered by another occurance of the same interrupt.
If your driver registers only one interrupt handler, then you can skip 
disabling the local interrupts - a deadlock is not possible.

You need _irqsave if the spinlock is shared between multiple instances 
of the hba, with different interrupts (i.e. it's possible that 
qla2x00_intr_handler is called for irq y while handling irq x).

--
    Manfred





^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4).
@ 2005-01-14 20:24 Eric Pesciotta
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Pesciotta @ 2005-01-14 20:24 UTC (permalink / raw)
  To: Linux-Scsi (E-mail)

I apologize in advance for the flash back.

>From Andrew Vasquez' reply on 2003-07-18 21:46:19 to Christoph Hellwig's comments
>>  - in qla2x00_queuecommand you need to call
>>   spin_unlock_irq/spin_lock_irq on the hostlock, otherwise your whole
>>  ->queuecommand runs with (local) interrupts disabled
>
>Noted..

The qla2x00 7.03.00 driver has exactly the same problem.
Is it safe to change qla2x00_queuecommand from spin_unlock(&io_request_lock) to spin_unlock_irq(&io_request_lock) in a 2.4 kernel?

-ejp-

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

end of thread, other threads:[~2005-01-14 20:26 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-17 23:40 [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4) Andrew Vasquez
2003-07-18 10:48 ` Christoph Hellwig
2003-07-18 11:23 ` Christoph Hellwig
2003-07-18 12:12   ` Lars Marowsky-Bree
2003-07-18 12:13     ` Christoph Hellwig
2003-07-18 12:26       ` Lars Marowsky-Bree
2003-07-18 12:26         ` Lars Marowsky-Bree
2003-07-18 12:34         ` Christoph Hellwig
2003-07-18 12:41           ` Lars Marowsky-Bree
2003-07-18 12:41             ` Lars Marowsky-Bree
2003-07-18 12:46             ` Christoph Hellwig
2003-07-18 13:46   ` Arjan van de Ven
2003-07-18 14:03     ` [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4 ) Jamie Wellnitz
2003-07-18 19:12     ` [ANNOUNCE] QLogic qla2xxx driver update available (v8.00.00b4) Christoph Hellwig
2003-07-18 19:12       ` Christoph Hellwig
2003-07-18 19:10   ` Christoph Hellwig
2003-07-18 21:15 Andrew Vasquez
2003-07-18 21:46 Andrew Vasquez
2003-07-18 21:53 Andrew Vasquez
2003-07-19  1:05 Manfred Spraul
2005-01-14 20:24 Eric Pesciotta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.