On 02/11/2010 05:19 PM, ravi.anand wrote:
On Feb 11, 2010, at 3:02 PM, Mike Christie wrote:
On 02/11/2010 04:57 PM, Mike Christie wrote:
On 02/11/2010 05:08 AM, Ravi Anand wrote:
On Mon, 01 Feb 2010, Mike Christie wrote:
On 01/30/2010 12:28 AM, Ravi Anand wrote:
- msleep(2000);
- } while (max_wait_time--);
+ if (got_ref&& (atomic_read(&rp->ref_count) == 1)) {
+ done++;
+ break;
+ }
+
+ msleep(ABORT_POLLING_PERIOD);
Did you want to use krefs for the refcounting?
We will add it to our to do list and submit a patch later on.
For right now we will like to stick to it as kref will require
additional testing.
And why is this so funky (got_ref arg and refcount peak) compared
to the
qla2xxx one?
I don't think qla2xxx is doing any reference counting in eh_abort()
path.
Basically its trying to differentiate for case where it takes an
additional
reference when the cmd is with the F/W. In that case if its the last
guy,
then it can go ahead and complete the command.
got_ref is always 0 isn't it (at least in the patch it is)? It seems
like you can just get rid of all that and just copy qla2xxx's code.
In the abort path (PATCH 10/11) it gets incremented. So its not
Ah I see.
always zero. And the reason we need a reference is because it can
get completed behind our back leading to a dangling srb. Since
now two guys are working on it : eh_abort() path as well as
the normal completion path.
Except for the code you added above, I am not seeing how
qla4xxx_eh_wait_on_command interacts with the srb or why it needs a
refcount on it. I do see why qla4xxx_abort_task needs it. If
qla4xxx_eh_wait_on_command does need the ref then it seems like the
device reset path is going to have a issue.
Can't you do sp_put() after qla4xxx_abort_task(). It seems like after
the qla4xxx_abort_task() call, then no one needs to touch the srb.
Without your patch, qla4xxx_eh_wait_on_command does not access the srb.
It only checks if the cmd has been completed by checking if SCp.ptr has
been cleared.
And why does qla4xxx_eh_abort() do this loop
+ /* Check active list for command */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ for (i = 1; i < MAX_SRBS; i++) {
+ srb_cmd = scsi_host_find_tag(ha->host, i);
+ if (srb_cmd == NULL)
+ continue;
It seems like you would want to do
qla4xxx_wait_for_hba_online()
spin_lock_irqsave(&ha->hardware_lock, flags);
srb = (struct srb *) cmd->SCp.ptr;
if (!srb)
/* raced while waiting for hba online */
return SUCCESS;
/* Get a reference to the sp and drop the lock.*/
sp_get(srb);
spin_unlock_irqrestore(&ha->hardware_lock, flags);
qla4xxx_abort_task()
sp_put(ha, srb)
spin_lock_irqsave(&ha->hardware_lock, flags);
qla4xxx_eh_wait_on_command(ha, cmd);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
