[PATCH] QLogic ISP2x00: remove needless busyloop

[PATCH] QLogic ISP2x00: remove needless busyloop

[Bjorn Helgaas]

There's no need to wait for an isp2x00 to recognize a fabric if
there's no isp2x00.  Probably nobody will notice the unnecessary
slowdown on real hardware, but it's a significant delay on a
simulator.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com.

===== drivers/scsi/qlogicfc.c 1.44 vs edited =====
--- 1.44/drivers/scsi/qlogicfc.c	2004-06-06 05:17:21 -06:00
+++ edited/drivers/scsi/qlogicfc.c	2004-08-09 12:23:06 -06:00
@@ -815,9 +815,11 @@
 	   some time before recognizing it is attached to a fabric */
 
 #if ISP2x00_FABRIC
-	for (wait_time = jiffies + 5 * HZ; time_before(jiffies, wait_time);) {
-		barrier();
-		cpu_relax();
+	if (hosts) {
+		for (wait_time = jiffies + 5 * HZ; time_before(jiffies, wait_time);) {
+			barrier();
+			cpu_relax();
+		}
 	}
 #endif
 

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Christoph Hellwig]

On Mon, Aug 09, 2004 at 12:52:58PM -0600, Bjorn Helgaas wrote:
> There's no need to wait for an isp2x00 to recognize a fabric if
> there's no isp2x00.  Probably nobody will notice the unnecessary
> slowdown on real hardware, but it's a significant delay on a
> simulator.

Don't use that driver.  qla2xxx is the right driver, and qlogicfc is only
still there and confuses people because davem is lazy and can shout louder
than others.

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Bjorn Helgaas]

On Monday 09 August 2004 2:03 pm, Christoph Hellwig wrote:
> On Mon, Aug 09, 2004 at 12:52:58PM -0600, Bjorn Helgaas wrote:
> > There's no need to wait for an isp2x00 to recognize a fabric if
> > there's no isp2x00.  Probably nobody will notice the unnecessary
> > slowdown on real hardware, but it's a significant delay on a
> > simulator.
> 
> Don't use that driver.  qla2xxx is the right driver, and qlogicfc is only
> still there and confuses people because davem is lazy and can shout louder
> than others.

I don't use it (note the "there's no isp2x00" bit above).  But it's
still part of ia64 defconfig (which I don't maintain), and it's easier
to tell people "use generic_defconfig" than to tell them to remove
CONFIG_SCSI_QLOGIC_FC by hand.

In general, I think if a driver is in the tree, it should be fair
game for bugfixes.  In fact, I see you did the most recent one to
qlogicfc :-)

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Christoph Hellwig]

On Mon, Aug 09, 2004 at 02:19:20PM -0600, Bjorn Helgaas wrote:
> I don't use it (note the "there's no isp2x00" bit above).  But it's
> still part of ia64 defconfig (which I don't maintain), and it's easier
> to tell people "use generic_defconfig" than to tell them to remove
> CONFIG_SCSI_QLOGIC_FC by hand.

So tell David to fix up the defconfig..

> In general, I think if a driver is in the tree, it should be fair
> game for bugfixes.  In fact, I see you did the most recent one to
> qlogicfc :-)

That wasn't a bugfix, look harder.

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Bjorn Helgaas]

> > In general, I think if a driver is in the tree, it should be fair
> > game for bugfixes.  In fact, I see you did the most recent one to
> > qlogicfc :-)
> 
> That wasn't a bugfix, look harder.

My apologies.  I should have written "if a driver is in the
tree, it should be fair game to improve it."

I think I'm missing something in this exchange.  Are you

1) Pointing out that the qlogicfc is obsolete and shouldn't
be used?  If so, thanks for the tip.

2) Suggesting that no changes to qlogicfc should be made?  If so,
I think it should be removed from the tree altogether.

3) Objecting to my patch because it is incorrect?  If so, please
give me a hint about what the problem is.

4) Something else?

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Christoph Hellwig]

On Mon, Aug 09, 2004 at 03:08:27PM -0600, Bjorn Helgaas wrote:
> > > In general, I think if a driver is in the tree, it should be fair
> > > game for bugfixes.  In fact, I see you did the most recent one to
> > > qlogicfc :-)
> > 
> > That wasn't a bugfix, look harder.
> 
> My apologies.  I should have written "if a driver is in the
> tree, it should be fair game to improve it."

I don't object to the patch.  But it was a _very_ _strong_ _hint_ that you
did something wrong if the patch actually matters for you.

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[David S. Miller]

On Mon, 9 Aug 2004 21:03:35 +0100
Christoph Hellwig <hch@infradead.org> wrote:

> Don't use that driver.  qla2xxx is the right driver, and qlogicfc is only
> still there and confuses people because davem is lazy and can shout louder
> than others.

Let me defend myself.

The qla2xxx driver numbers devices differently than qlogicfc does.

The qla2xxx driver never existed upstream until it was added a few
months ago, and even with that it's 2.6.x only, therefore people
using pure upstream kernels for all of their systems (such as myself)
has only the qlogicfc driver to use.

So it's not how loud I can yell or not, it's that not keeping the
qlogicfc driver around would break people's systems.

You could remove the qlogicfc driver if you really wanted, by providing
a config option that would provide qlogicfc compatible device numbering
in the qla2xxx driver.

So implement that instead of bitching about me trying to keep people's
systems functional, ok? :-)

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Christoph Hellwig]

On Mon, Aug 09, 2004 at 02:11:55PM -0700, David S. Miller wrote:
> You could remove the qlogicfc driver if you really wanted, by providing
> a config option that would provide qlogicfc compatible device numbering
> in the qla2xxx driver.

It's called CONFIG_DEVFS.  disable this config option (it's marked OBSOLETE anyway)
and your device names are the same.

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[David S. Miller]

On Mon, 9 Aug 2004 22:15:29 +0100
Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Aug 09, 2004 at 02:11:55PM -0700, David S. Miller wrote:
> > You could remove the qlogicfc driver if you really wanted, by providing
> > a config option that would provide qlogicfc compatible device numbering
> > in the qla2xxx driver.
> 
> It's called CONFIG_DEVFS.  disable this config option (it's marked OBSOLETE anyway)
> and your device names are the same.

I wish to do 2.4.x and 2.6.x development on the same system.
Is this such a foreign concept for you?

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Christoph Hellwig]

On Mon, Aug 09, 2004 at 02:20:12PM -0700, David S. Miller wrote:
> > It's called CONFIG_DEVFS.  disable this config option (it's marked OBSOLETE anyway)
> > and your device names are the same.
> 
> I wish to do 2.4.x and 2.6.x development on the same system.
> Is this such a foreign concept for you?

In 2.4 devfs was even more broken.   In fact it's whole scsi naming scheme is
complete bullshit, and you're now complaining because of that.  That has absolutely
nothing to do with 2.4 vs 2.6.

Re: [PATCH] QLogic ISP2x00: remove needless busyloop

[Alan Cox]

On Llu, 2004-08-09 at 22:11, David S. Miller wrote:
> You could remove the qlogicfc driver if you really wanted, by providing
> a config option that would provide qlogicfc compatible device numbering
> in the qla2xxx driver.
> 
> So implement that instead of bitching about me trying to keep people's
> systems functional, ok? :-)

Easier to just mark it OBSOLETE and take it out in a year or so