Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()

2007-03-13 Thread Brian King
Tejun Heo wrote:
 Association to SCSI host is done via pointer now even for native ATA
 case, so this should be easier for SAS.  What I'm worried about is how
 EH gets invoked.  libata depends on EH to do a lot of things including
 probing, requesting sense data, etc.  How should this work?

For SAS, the scsi_host pointer in the ata port is NULL today, since libata
is really not managing the scsi host, the LLDD is. I think the initialization
model we want for SAS is a little different than the one you are heading
towards on SATA. For SAS, I think we just want to be able to alloc/init
and delete/destroy a SATA device a they show up on the transport,
without tying it to initialization of the ata host. And this set of
patches doesn't necessarily prevent that...

 SAS attached libata port shares EH with the SAS SCSI host, right?  How can

Right.

 we connect SAS EH with libata EH and would it be okay for libata EH hold
 the SCSI EH (thus holding all command execution on the host) to handle
 ATA exceptions?

Currently, ipr calls ata_do_eh from its eh_device_reset_handler function.
This seems to work well enough with the testing that I've done, but it
would certainly be nice to get to a more layered EH approach, where we
could possibly have pluggable error handlers for different device types.

Regarding holding all command execution on the host while performing eh,
that doesn't seem to be a huge issue from my perspective, not sure if
this would have a larger negative impact on others... Generally speaking,
we shouldn't be entering eh very often, and it should only be happening
if something went wrong. The biggest issue here might be ATAPI devices,
since they tend to report more errors during normal running. The request
sense for these devices for SAS is done without entering eh today. Would
you want to move this into eh as well?

Brian


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()

2007-03-13 Thread Tejun Heo

Hello, Brian.

Brian King wrote:

Tejun Heo wrote:

Association to SCSI host is done via pointer now even for native ATA
case, so this should be easier for SAS.  What I'm worried about is how
EH gets invoked.  libata depends on EH to do a lot of things including
probing, requesting sense data, etc.  How should this work?


For SAS, the scsi_host pointer in the ata port is NULL today, since libata
is really not managing the scsi host, the LLDD is. I think the initialization
model we want for SAS is a little different than the one you are heading
towards on SATA. For SAS, I think we just want to be able to alloc/init
and delete/destroy a SATA device a they show up on the transport,
without tying it to initialization of the ata host. And this set of
patches doesn't necessarily prevent that...


Yeap, I tried to keep SAS bridge functions working.  If SAS doesn't need 
the host abstraction and wanna do stuff per-port basis, ata_port_alloc() 
can be directly exported and separating out per-port register routine 
shouldn't be too difficult, but I do think it would still be beneficial 
to have ata_host structure in SAS case too for code simplicity if not 
for anything else.



SAS attached libata port shares EH with the SAS SCSI host, right?  How can


Right.


we connect SAS EH with libata EH and would it be okay for libata EH hold
the SCSI EH (thus holding all command execution on the host) to handle
ATA exceptions?


Currently, ipr calls ata_do_eh from its eh_device_reset_handler function.
This seems to work well enough with the testing that I've done, but it
would certainly be nice to get to a more layered EH approach, where we
could possibly have pluggable error handlers for different device types.


That's an unexpected usage of ata_do_eh() but I can see how that works 
and using ata_do_eh() for that purpose actually makes sense.  Most SCSI 
related dancing is done before and after ata_do_eh() and ata_do_eh() 
only deals with ATA qc's (except for scsi_eh_finish_cmd() called to 
finish failed qc's but these are still for only scmds associated with qcs).


In the future, we might need to separate those direct 
scsi_eh_finish_cmd() calls out of ata_do_eh() so that ata_do_eh() really 
deals with libata qc proper but that change shouldn't be too difficult 
for SAS.



Regarding holding all command execution on the host while performing eh,
that doesn't seem to be a huge issue from my perspective, not sure if
this would have a larger negative impact on others... Generally speaking,
we shouldn't be entering eh very often, and it should only be happening
if something went wrong. The biggest issue here might be ATAPI devices,
since they tend to report more errors during normal running. The request
sense for these devices for SAS is done without entering eh today. Would
you want to move this into eh as well?


No, not for SAS.  The reasons why I put sense requesting to EH were...

1. to make fast path code straight forward (no qc reusing dance)

2. in native ATA, we have per-port EH thread so sharing is not a problem.

As #2 is not true in SAS case, I think keeping sense requesting out of 
EH is the right thing to do here.  I still think that it's much 
simpler/reliable to handle any exception case in a separate thread.  I 
think this in the long term should be solved by making EH per-request 
queue (we of course will need mechanism to synchronize several EHs so 
that we can take host-wide EH actions).


Thanks.

--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()

2007-03-12 Thread Brian King
Tejun Heo wrote:
 * ipr is now the only user of ata_host_init().  Either kill it by
   converting ipr to use ata_host_alloc() and friends or rename and
   move it to libata-scsi.c

One of the problems with converting ipr to use ata_host_alloc and
friends is that it then forces ipr to tell libata how many SATA ports
are possible. On SAS, this number can't really be calculated, since
the maximum number of SATA devices which can possibly be cabled to a
SAS adapter, particularly with SAS expanders, is a very large number
and is not practical for how this is being used in the current
implementation. My guess is that aic94xx will have similar issues/concerns.

Brian

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()

2007-03-12 Thread Tejun Heo
Hello, Brian.

Brian King wrote:
 Tejun Heo wrote:
 * ipr is now the only user of ata_host_init().  Either kill it by
   converting ipr to use ata_host_alloc() and friends or rename and
   move it to libata-scsi.c
 
 One of the problems with converting ipr to use ata_host_alloc and
 friends is that it then forces ipr to tell libata how many SATA ports
 are possible. On SAS, this number can't really be calculated, since
 the maximum number of SATA devices which can possibly be cabled to a
 SAS adapter, particularly with SAS expanders, is a very large number
 and is not practical for how this is being used in the current
 implementation. My guess is that aic94xx will have similar issues/concerns.

I dunno much about SAS integration, so I intentionally left it alone, so
this patchset shouldn't change behavior from SAS's point of view.
Making host-ports dynamic isn't difficult at all.  We can just separate
out the ports[] array and make it rcu and supply a few access helpers so
that we can adjust its size dynamically.

Association to SCSI host is done via pointer now even for native ATA
case, so this should be easier for SAS.  What I'm worried about is how
EH gets invoked.  libata depends on EH to do a lot of things including
probing, requesting sense data, etc.  How should this work?  SAS
attached libata port shares EH with the SAS SCSI host, right?  How can
we connect SAS EH with libata EH and would it be okay for libata EH hold
the SCSI EH (thus holding all command execution on the host) to handle
ATA exceptions?

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()

2007-03-09 Thread Jeff Garzik

Tejun Heo wrote:

Reorganize ata_host_alloc() and its subroutines into the following
three functions.

* ata_host_alloc() : allocates host and its attached ports.  shost is
  not attached automatically.

* ata_scsi_add_hosts() : allocates and adds shosts associated with an
  ATA host.  Used by ata_host_attach().

* ata_host_attach() : takes a fully initialized ata_host structure and
  registers it to libata layer and probes it.


I think it's more common in Linux to use foo_register than foo_attach. 
attach is more a BSD-ism :)


ACK patches 2-3, modulo the naming comment



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html