diff mbox

[ovs-dev] windows, python: set the reset event to automatic

Message ID 1503398837-2498-3-git-send-email-abalutoiu@cloudbasesolutions.com
State Superseded
Headers show

Commit Message

Alin Balutoiu Aug. 22, 2017, 10:47 a.m. UTC
The overlapped structures used for read and write operations
get an event with manual reset flag set on True.

At the moment events are waiting to be reset with their
state being always signaled.

This commit sets from manual reset to automatic reset
for the events on the overlapped read/write structures.

Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
---
 python/ovs/stream.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Russell Bryant Aug. 22, 2017, 12:34 p.m. UTC | #1
On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu
<abalutoiu@cloudbasesolutions.com> wrote:
> The overlapped structures used for read and write operations
> get an event with manual reset flag set on True.
>
> At the moment events are waiting to be reset with their
> state being always signaled.

Just making sure I understand ... once an event was raised for a fd,
it was never reset?  What was the side effect?  Did it turn the event
loop into a busy loop?

> This commit sets from manual reset to automatic reset
> for the events on the overlapped read/write structures.
>
> Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
> ---
>  python/ovs/stream.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index f82a449..b30c4aa 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -103,9 +103,11 @@ class Stream(object):
>          self.pipe = pipe
>          if sys.platform == 'win32':
>              self._read = pywintypes.OVERLAPPED()
> -            self._read.hEvent = winutils.get_new_event()
> +            self._read.hEvent = winutils.get_new_event(bManualReset=False,
> +                                                       bInitialState=False)
>              self._write = pywintypes.OVERLAPPED()
> -            self._write.hEvent = winutils.get_new_event()
> +            self._write.hEvent = winutils.get_new_event(bManualReset=False,
> +                                                        bInitialState=False)
>              if pipe is not None:
>                  # Flag to check if fd is a server HANDLE.  In the case of a
>                  # server handle we have to issue a disconnect before closing
> --
> 2.10.0.windows.1
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Alin Balutoiu Aug. 22, 2017, 12:41 p.m. UTC | #2
Comments answered inline.

Thanks,
Alin Balutoiu.

> -----Original Message-----
> From: Russell Bryant [mailto:russell@ovn.org]
> Sent: Tuesday, August 22, 2017 2:35 PM
> To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] windows, python: set the reset event to
> automatic
> 
> On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu
> <abalutoiu@cloudbasesolutions.com> wrote:
> > The overlapped structures used for read and write operations get an
> > event with manual reset flag set on True.
> >
> > At the moment events are waiting to be reset with their state being
> > always signaled.
> 
> Just making sure I understand ... once an event was raised for a fd, it was
> never reset?  What was the side effect?  Did it turn the event loop into a
> busy loop?
[Alin Balutoiu] The side effect was observed in the OpenStack case when
using the native interface driver. Since the event was never reset, the
call to poller.block() always was returning the same event, even though
there was no read operation to be done. This resulted in having an infinite
loop consuming a lot of CPU.
> 
> > This commit sets from manual reset to automatic reset for the events
> > on the overlapped read/write structures.
> >
> > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
> > ---
> >  python/ovs/stream.py | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/ovs/stream.py b/python/ovs/stream.py index
> > f82a449..b30c4aa 100644
> > --- a/python/ovs/stream.py
> > +++ b/python/ovs/stream.py
> > @@ -103,9 +103,11 @@ class Stream(object):
> >          self.pipe = pipe
> >          if sys.platform == 'win32':
> >              self._read = pywintypes.OVERLAPPED()
> > -            self._read.hEvent = winutils.get_new_event()
> > +            self._read.hEvent = winutils.get_new_event(bManualReset=False,
> > +
> > + bInitialState=False)
> >              self._write = pywintypes.OVERLAPPED()
> > -            self._write.hEvent = winutils.get_new_event()
> > +            self._write.hEvent = winutils.get_new_event(bManualReset=False,
> > +
> > + bInitialState=False)
> >              if pipe is not None:
> >                  # Flag to check if fd is a server HANDLE.  In the case of a
> >                  # server handle we have to issue a disconnect before
> > closing
> > --
> > 2.10.0.windows.1
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> 
> 
> --
> Russell Bryant
Russell Bryant Aug. 22, 2017, 3:34 p.m. UTC | #3
On Tue, Aug 22, 2017 at 8:41 AM, Alin Balutoiu
<abalutoiu@cloudbasesolutions.com> wrote:
> Comments answered inline.
>
> Thanks,
> Alin Balutoiu.
>
>> -----Original Message-----
>> From: Russell Bryant [mailto:russell@ovn.org]
>> Sent: Tuesday, August 22, 2017 2:35 PM
>> To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
>> Cc: dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] windows, python: set the reset event to
>> automatic
>>
>> On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu
>> <abalutoiu@cloudbasesolutions.com> wrote:
>> > The overlapped structures used for read and write operations get an
>> > event with manual reset flag set on True.
>> >
>> > At the moment events are waiting to be reset with their state being
>> > always signaled.
>>
>> Just making sure I understand ... once an event was raised for a fd, it was
>> never reset?  What was the side effect?  Did it turn the event loop into a
>> busy loop?
> [Alin Balutoiu] The side effect was observed in the OpenStack case when
> using the native interface driver. Since the event was never reset, the
> call to poller.block() always was returning the same event, even though
> there was no read operation to be done. This resulted in having an infinite
> loop consuming a lot of CPU.

Thanks, that's what I thought, just making sure I understood properly.

Acked-by: Russell Bryant <russell@ovn.org>

>>
>> > This commit sets from manual reset to automatic reset for the events
>> > on the overlapped read/write structures.
>> >
>> > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
>> > ---
>> >  python/ovs/stream.py | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/python/ovs/stream.py b/python/ovs/stream.py index
>> > f82a449..b30c4aa 100644
>> > --- a/python/ovs/stream.py
>> > +++ b/python/ovs/stream.py
>> > @@ -103,9 +103,11 @@ class Stream(object):
>> >          self.pipe = pipe
>> >          if sys.platform == 'win32':
>> >              self._read = pywintypes.OVERLAPPED()
>> > -            self._read.hEvent = winutils.get_new_event()
>> > +            self._read.hEvent = winutils.get_new_event(bManualReset=False,
>> > +
>> > + bInitialState=False)
>> >              self._write = pywintypes.OVERLAPPED()
>> > -            self._write.hEvent = winutils.get_new_event()
>> > +            self._write.hEvent = winutils.get_new_event(bManualReset=False,
>> > +
>> > + bInitialState=False)
>> >              if pipe is not None:
>> >                  # Flag to check if fd is a server HANDLE.  In the case of a
>> >                  # server handle we have to issue a disconnect before
>> > closing
>> > --
>> > 2.10.0.windows.1
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
>>
>> --
>> Russell Bryant
diff mbox

Patch

diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index f82a449..b30c4aa 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -103,9 +103,11 @@  class Stream(object):
         self.pipe = pipe
         if sys.platform == 'win32':
             self._read = pywintypes.OVERLAPPED()
-            self._read.hEvent = winutils.get_new_event()
+            self._read.hEvent = winutils.get_new_event(bManualReset=False,
+                                                       bInitialState=False)
             self._write = pywintypes.OVERLAPPED()
-            self._write.hEvent = winutils.get_new_event()
+            self._write.hEvent = winutils.get_new_event(bManualReset=False,
+                                                        bInitialState=False)
             if pipe is not None:
                 # Flag to check if fd is a server HANDLE.  In the case of a
                 # server handle we have to issue a disconnect before closing