[ovs-dev] [PATCH] datapath: Avoid system freeze due to ovs-flow-rehash softlockup.

Pravin Shelar pshelar at nicira.com
Thu May 24 00:11:30 UTC 2012


On Wed, May 23, 2012 at 5:03 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Wed, May 23, 2012 at 10:36 AM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> OVS datapath does periodic flow table rehash which takes genl_lock
>> in workq context.
>> In some cases, like ports add or delete, genl_lock can cause softlockup
>> as vswitchd would take and succeed with genl_lock and rehash workq
>> would block on the lock. Eventually rehash will proceed, flow rehash
>> is low priority task so this is not problem for rehashing.
>> But it is blocking workq thread; some other workq item from other
>> kernel subsystem would be blocked and can cause system freeze.
>> To avoid workq blocking and system freeze, we can use OVS compat workq.
>> It runs in separate kernel thread thus does not block any non-ovs
>> deferred workq work item.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>
> Can you add a note in the commit message about how we're planning on
> addressing this in the future?
>
ok

> I got a whitespace error:
>
> Applying: datapath: Avoid system freeze due to ovs-flow-rehash softlockup.
> /home/jesse/openvswitch/.git/rebase-apply/patch:64: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
>> diff --git a/datapath/linux/compat/include/linux/workqueue.h b/datapath/linux/compat/include/linux/workqueue.h
>> index 919afe3..e5e2178 100644
>> --- a/datapath/linux/compat/include/linux/workqueue.h
>> +++ b/datapath/linux/compat/include/linux/workqueue.h
>> @@ -68,6 +64,7 @@ int cancel_delayed_work_sync(struct delayed_work *dwork);
>>                (_work)->func = (_func);                        \
>>        } while (0)
>>
>> -#endif /* kernel version < 2.6.23 */
>> +
>> +extern void flush_scheduled_work(void);
>
> Is this declaration necessary?  I didn't find any other references to it.

In new kernel it is declared in workqueue.h and used in interrupt.h,
thats why we just need declaration.



More information about the dev mailing list