Hi, Luben.
Luben Tuikov wrote:
On 08/29/05 05:14, Tejun Heo wrote:
Both all the list-heads need to be cleared, otherwise there may be list
corruption next time the element is added to the list_head.
scmd->eh_entry is never used as list head. It's always used as list
entry. So, technically, it needs not be cleared, I think. No? The
problem we had was w/ shost->eh_cmd_q not being cleared.
In your "strategy" routine:
...
spin_lock_irqsave(shost->host_lock, flags);
list_splice_init(&shost->eh_cmd_q, &error_q);
spin_unlock_irqrestore(shost->host_lock, flags);
...
loop {
...
list_del_init(&cmd->eh_entry);
...
}
A good policy to follow is:
1. Never leave prev/next pointing somewhere where
- you don't belong, or
- where you don't know existance is in place.
2. Someone (memory release?) may do:
if (!list_empty(cmd->eh_entry))
Refuse to free the memory.
Which is often the case to check if the object belongs to
a list. (You shouldn't have to do this but case pointed only for
illustrational purposes.)
Luben
The reason why I explicitly stated that clearing scmd->eh_entry was
not currently necessary was that libata had infinite loop bug due to
eh_cmd_q related memory corruption and I wanted to make sure that not
clearing scmd->eh_entry wasn't the cause.
Previously, libata didn't clear both shost->eh_cmd_q and
scmd->eh_entry. I posted an one liner which cleared shost->eh_cmd_q and
I believe that's the fix for the problem. However, Mark Lord is still
having lockup problems with libata which, I suspect, is because libata
doesn't handle PM's properly. But, as I'm not very sure, I wanted to
make sure that libata's not clearing scmd->eh_cmd_q is not causing the
lockup.
I agree that, as a policy, always clearing list_head's are nice if
it's not in the *real* hot path where reducing several assignments
matter, but as it's not strictly/technically necessary, it might be
difficult to enforce as long as functions which don't clear list_head
are there.
Thanks for your input. :-)
--
tejun
-
: 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
