Hi Alin, I have the following concern with your change around the below code:
KeWaitForSingleObject(&context->event, Executive, KernelMode,
FALSE, (LARGE_INTEGER *)&timeout);
When we call into KeWaitForSingleObject, we want to wait indefinitely till we
receive a wakeup event. Your patch simply increases the timeout from 0 to
OVS_IPNEIGH_TIMEOUT. This will end up waking up the IpHelper thread even if no
wakeup event is received once the timeout value expires. We have seen cases
where simply looping in this thread unnecessarily increases the CPU consumed.
Hence, we should only wake up this thread if an event is received to wake it up.
Please refer to my change and see if you can incorporate this into your change.
Thanks,
Shashank
________________________________
From: Alin Serdean <[email protected]>
Sent: Wednesday, November 23, 2016 1:14:39 PM
To: Yin Lin; Shashank Ram; Nithin Raju; Sairam Venugopal
Cc: [email protected]
Subject: RE: [ovs-dev] [PATCH v2] datapath-windows: Avoid busy wait in
OvsStartIpHelper
Hi Shashank,
Thanks a lot for the patch!
My only concern is that this issue has already been addresed in one of the
multiple port patches:
https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_669598_&d=DgIFAg&c=uilaK90D4TOVoH58JNXRgQ&r=6OuVHk-mnufSWzkKa74UkQ&m=1I78-nNpLSZav-yhv-wSNAtXMaiyp6lciwose971dBE&s=Sfy280kZ0O4q2BTI3Aa4MlG4HSKpDVrPH5koGzTLKmQ&e=
I plan to respin the series until the end of the week.
Thanks,
Alin.
> -----Original Message-----
> From: [email protected] [mailto:ovs-dev-
> [email protected]] On Behalf Of Yin Lin
> Sent: Wednesday, November 23, 2016 5:58 AM
> To: Shashank Ram <[email protected]>
> Cc: [email protected]
> Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Avoid busy wait in
> OvsStartIpHelper
>
> Acked-by: Yin Lin<[email protected]>
>
> Best regards,
> Yin
>
> On Tue, Nov 22, 2016 at 5:32 PM, Shashank Ram <[email protected]>
> wrote:
>
> > Previously, the IP Helper thread would wait for an event but with a
> > timeout of 0, which resulted in the thread busy waiting causing high
> > CPU usage by the kernel.
> > Since the IP Helper thread is only required based on certain events,
> > it makes sense to wait indefinitely till we receieve such an event
> > notification to wake up the thread. This change aims to address this
> > issue.
> >
> > When OvsEnqueueIpHelperRequest() or OvsInternalAdapterUp() is called,
> > the ovsNumIpHelperRequests counter is incremented, but upon
> > consumption of the request, is not decremented.
> > Since the wakeup logic for the thread is determined by this counter
> > value, we need to reset the counter back correctly once the request
> > has been consumed by the IP Helper thread.
> >
> > Signed-off-by: Shashank Ram <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
