On Wed, May 23, 2012 at 5:16 PM, Jesse Gross <je...@nicira.com> wrote: > On Wed, May 23, 2012 at 5:11 PM, Pravin Shelar <pshe...@nicira.com> wrote: >> On Wed, May 23, 2012 at 5:03 PM, Jesse Gross <je...@nicira.com> wrote: >>> On Wed, May 23, 2012 at 10:36 AM, Pravin B Shelar <pshe...@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 <pshe...@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. > > OK, thanks. > > Acked-by: Jesse Gross <je...@nicira.com>
Thanks, Pushed to master. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
