Re: [PATCH] helper for device list traversal

Re: [PATCH] helper for device list traversal

[Geert Uytterhoeven]

On Thu, 25 Sep 2003, Linux Kernel Mailing List wrote:
> ChangeSet 1.1217.10.16, 2003/09/25 12:10:17-05:00, hch@lst.de
> 
> 	[PATCH] helper for device list traversal
> 	
> 	This patch adds shost_for_each_device().  It's used to abstract out
> 	scsi_host.my_devices traversal.  The next step will be to replace
> 	the current simple implementation with one that's fully locked down
> 	an reference counted.

Is this what we should use to fix the currently broken list traversal[*] in
drivers/scsi/{a2091,gvp11,53c7xx}.c?

Currently ut uses

    struct Scsi_Host *instance;
    for (instance = first_instance; instance &&
	 instance->hostt == xxx_template; instance = instance->next)

bust Scsi_Host.next was removed a while ago...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] helper for device list traversal

[Christoph Hellwig]

On Sat, Sep 27, 2003 at 01:34:15PM +0200, Geert Uytterhoeven wrote:
> Is this what we should use to fix the currently broken list traversal[*] in
> drivers/scsi/{a2091,gvp11,53c7xx}.c?

where does that [*] refere to?

> Currently ut uses
> 
>     struct Scsi_Host *instance;
>     for (instance = first_instance; instance &&
> 	 instance->hostt == xxx_template; instance = instance->next)
> 
> bust Scsi_Host.next was removed a while ago...

Most users of that construct want to use scsi_lookup_host (if they
looked for a specific host struct).

Above patch is for scsi_device list traversal, not Scsi_Host.

Re: [PATCH] helper for device list traversal

[Geert Uytterhoeven]

On Sat, 27 Sep 2003, Christoph Hellwig wrote:
> On Sat, Sep 27, 2003 at 01:34:15PM +0200, Geert Uytterhoeven wrote:
> > Is this what we should use to fix the currently broken list traversal[*] in
> > drivers/scsi/{a2091,gvp11,53c7xx}.c?
> 
> where does that [*] refere to?

To the part below. Sorry, I changed the structure of the mail but forgot to
remove the [*].

> > Currently ut uses
> > 
> >     struct Scsi_Host *instance;
> >     for (instance = first_instance; instance &&
> > 	 instance->hostt == xxx_template; instance = instance->next)
> > 
> > bust Scsi_Host.next was removed a while ago...
> 
> Most users of that construct want to use scsi_lookup_host (if they
> looked for a specific host struct).

The A2091 and GVP-II drivers want to traverse the list of SCSI host adapters in
their interrupt handler, to find their Scsi_Host instances and check whether
the (shared) interrupt was meant for one of them.

An alternative would be to register the interrupt handler multiple times and
use the Scsi_Host instance pointer as the interrupt handler data pointer.

For 53c7xx it's a bit more complex, I don't know anything about its internals.

In all 3 cases, I don't have the hardware...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] helper for device list traversal

[Christoph Hellwig]

On Sat, Sep 27, 2003 at 01:34:15PM +0200, Geert Uytterhoeven wrote:
> On Thu, 25 Sep 2003, Linux Kernel Mailing List wrote:
> > ChangeSet 1.1217.10.16, 2003/09/25 12:10:17-05:00, hch@lst.de
> > 
> > 	[PATCH] helper for device list traversal
> > 	
> > 	This patch adds shost_for_each_device().  It's used to abstract out
> > 	scsi_host.my_devices traversal.  The next step will be to replace
> > 	the current simple implementation with one that's fully locked down
> > 	an reference counted.
> 
> Is this what we should use to fix the currently broken list traversal[*] in
> drivers/scsi/{a2091,gvp11,53c7xx}.c?
> 
> Currently ut uses
> 
>     struct Scsi_Host *instance;
>     for (instance = first_instance; instance &&
> 	 instance->hostt == xxx_template; instance = instance->next)
> 
> bust Scsi_Host.next was removed a while ago...

Urgg.  This is totally hosed.  The drivers should be fixed up to
pass the right host to request_irq and the only handle the right
one in the isr instead of walking a lits of all hosts of that template..

Re: [PATCH] helper for device list traversal

[Christoph Hellwig]

On Sat, Sep 27, 2003 at 02:15:56PM +0200, Geert Uytterhoeven wrote:
> The A2091 and GVP-II drivers want to traverse the list of SCSI host adapters in
> their interrupt handler, to find their Scsi_Host instances and check whether
> the (shared) interrupt was meant for one of them.
> 
> An alternative would be to register the interrupt handler multiple times and
> use the Scsi_Host instance pointer as the interrupt handler data pointer.

Yes, they should do that.

> For 53c7xx it's a bit more complex, I don't know anything about its internals.

The occurance in #if 0 code should probably use scsi_lookup_host.
process_issue_queue needs to be rewritten to be per-host.