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. -- 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
