Hi,
fx chen <chenfx2014@xxxxxxxxx> writes:
> But carefully consider your modify，Logically it is not easy to
> understand，only fix current bug(Currently, I'm not sure whether still
> potential problems such modifications ),
Yep, I like your last version of the fix more than mine. Moving c_hlist
unlinking completely into iscsi_free_task is more logical than doing it
there "just in case" AND leaving it in iscsi_free_cmd_task.
As of c_siblings and c_list chains, they are supposed to be managed
correctly, so I'd better avoid adding an over-protective check. As of
c_siblings, its life cycle is simple and it's obviously correct (for any
task, link when allocated, unlink when freed), as of c_list, I'm not so
sure, but that's no reason to patch iscsi_free_task ahead of time.
It would be great if you'd get time to test with a following fragment in
iscsi_free_task:
if (task->c_list.prev &&
task->c_list.next && !list_empty(&task->c_list)) {
abort("Freeing a task with non-empty c_list");
}
>
> Iwe carefully consider the logical，why do below task->c_hlist.prev
> task->c_hlist.prev check in iscsi_free_task + if (task->c_hlist.prev
> && task->c_hlist.next + && !list_empty(&task->c_hlist)) { +
> list_del(&task->c_hlist); + } + Similarly，besides task->c_hlist，
> task have task->c_siblings, Why not do below task->c_siblings relate
> check? + if (task->c_siblings.prev && task->c_siblings.next + &&
> !list_empty(&task->c_siblings)) { + list_del(&task->c_siblings); + } +
> Similarly，besides task->c_hlist， task have task->c_list, Why not do
> below task->c_list relate check? + if (task->c_list.prev &&
> task->c_list.next + && !list_empty(&task->c_list)) { +
> list_del(&task->c_list); + } +
--
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
