Re: fix stgt crash in conn_close

Hi,

fx chen <[email protected]> writes:

> But carefully consider your modify=EF=BC=8CLogically it is not easy t=
o
> understand=EF=BC=8Conly fix current bug(Currently, I'm not sure wheth=
er 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 an=
y
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 i=
n
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");
}



>=20
> Iwe carefully consider the logical=EF=BC=8Cwhy do below task->c_hlist=
=2Eprev
> 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=EF=BC=8Cbesides task->c_hli=
st=EF=BC=8C
> 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=EF=BC=8Cbesides task->c_hlist=EF=BC=8C task have task->c_li=
st, 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); + } +

--=20
Regards, Anton Kovalenko | +7(916)345-34-02 | Elektrostal' MO, Russia