Re: [PATCH 1/1] iscsi: fix regression caused by session lock patch

[Mike Christie <michaelc@cs.wisc.edu>]

On 11/13/2015 09:06 AM, Or Gerlitz wrote:
>> The patch has caused multiple regressions, did not even compile when
>> > sent to me, and was poorly reviewed and I have not heard from you guys
>> > in a week. Given the issues the patch has had and the current time, I do
>> > not feel comfortable with it anymore. I want to re-review it and fix it
>> > up when there is more time.
> Mike (Hi),
> 
> It's a complex patch that touches all the iscsi transports, and yes,
> when it was send to you the 1st time, there was build error on one of
> the offload transports (bad! but happens) and yes, as you pointed, one
> static checker fix + one bug fix for it went upstream after this has
> been merged, happens too.

A patch should not cause this many issues.

> What makes you say it was poorly reviewed?

I just did not do a good job at looking at the patch. I should have
caught all of these issues.

- The bnx2i cleanup_task bug should have been obvious, especially for me
because I had commented about the back lock and the abort path.

- This oops, was so basic. Incorrect locking around a linked list being
accessed from 2 threads is really one of those 1st year kernel
programmer things.

- The iscsi_xmit_task static checker locking was really simple too.

- There was the issue offlist I emailed you guys about where I started
to try and fix/review it yesterday, and I found another race in the
abort and completion path that can result in a null pointer reference.

- I have not had time to fully review it again, but I think the the
reset/recovery code looks shady too.

> 
> And now after few years in upstream a possibly real bug was found
> (happens), why rush and revert? lets see if we can fix.

Send patches.