Re: [PATCH] RDMA/cxgb4: don't declare wr_wait objects on the stack.

Steve Wise
2011-05-16T16:05:01Z

On 05/16/2011 10:56 AM, Steve Wise wrote:
> On 05/16/2011 10:53 AM, Roland Dreier wrote:
>> On Mon, May 16, 2011 at 8:18 AM, Steve Wise<swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org> wrote:
>>> Roland, I need to recall this patch. It appears to have some problem.
>> Good thing I didn't apply it yet.
>>
>> I did think it should be possible to declare wait objects on the stack... the
>> origin of completions was to handle exactly that issue; the older semaphore
>> structure was vulnerable to the wakeup after free, but completions handle
>> that. But I didn't look at the cxgb4 code at all...
>
> I'm pretty sure the race exists as described in my commit comment. And I don't see how the wait object code could
> avoid this issue. Unless I'm just all wrong. :)
>
>

FYI:

So the crash I kept seeing was that my wake up path would end up accessing a freed wait object. I added a 100us udelay
in this path just after setting the wait condition to true, but before calling wake_up(). With this delay, I hit the
crash much more quickly. So in my mind I proved that the race exists. I then added this commit to get rid of on-stack
wait objects, but left the 100us delay in my code and tested that the crash goes away. So I proved, in my mind, that 1)
this was a real issue, and 2) my fix worked. I then ran some regression tests to convince myself the fix was stable
before posting the patch on Friday. However our QA seems to be seeing some issues with this patch on various backports
with older kernels, so I need to investigate this further. Thus I recalled the patch.