[ovs-dev] lock ordering from Valgrind --tool=helgrind

William Tu u9012063 at gmail.com
Sat Apr 23 00:52:27 UTC 2016


Hi Ben,

Thank you! I will continue working on other fixes.

Regards,
William

On Fri, Apr 22, 2016 at 5:05 PM, Ben Pfaff <blp at ovn.org> wrote:

> I sent a series that should fix this deadlock.  You can still fix (or
> report) others!
>
> http://openvswitch.org/pipermail/dev/2016-April/070055.html
> http://openvswitch.org/pipermail/dev/2016-April/070056.html
>
> On Tue, Apr 12, 2016 at 10:05:36PM -0700, William Tu wrote:
> > Let me take a stab at them first, if I couldn't figure out, I will pass
> > them along. Thanks
> >
> > On Tue, Apr 12, 2016 at 9:09 PM, Ben Pfaff <blp at ovn.org> wrote:
> >
> > > On Tue, Apr 12, 2016 at 02:39:49PM -0700, William Tu wrote:
> > > > Hi Ben,
> > > >
> > > > Thanks, the fix solves the problem.
> > > >
> > > > However, there are a couple of errors reported by Helgrind, do you
> think
> > > > it's worth fixing all of them?
> > >
> > > Probably.  Do you want to pass them along for me to take a look?  Or do
> > > you want to take a stab at them first?
> > >
> > > > Regards,
> > > > William
> > > >
> > > >
> > > > On Sun, Apr 10, 2016 at 2:16 PM, Ben Pfaff <blp at ovn.org> wrote:
> > > >
> > > > > On Wed, Feb 24, 2016 at 12:40:33PM -0800, William Tu wrote:
> > > > > > These two locks are reported by helgrind which have ordering
> > > > > inconsistency.
> > > > > > "netdev_class_mutex"
> > > > > > "route_table_mutex"
> > > > > >
> http://openvswitch.org/pipermail/discuss/2016-February/020216.html
> > > > >
> > > > > Thanks for the report.  Somehow I didn't get the original report,
> > > > > although I see it in the archive.
> > > > >
> > > > > Does the following appear to fix the problem?
> > > > >
> > > > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > > > > index e398562..b3eef5d 100644
> > > > > --- a/lib/netdev-vport.c
> > > > > +++ b/lib/netdev-vport.c
> > > > > @@ -1,5 +1,5 @@
> > > > >  /*
> > > > > - * Copyright (c) 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> > > > > + * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
> > > > >   *
> > > > >   * Licensed under the Apache License, Version 2.0 (the "License");
> > > > >   * you may not use this file except in compliance with the
> License.
> > > > > @@ -373,7 +373,6 @@ netdev_vport_run(void)
> > > > >  {
> > > > >      uint64_t seq;
> > > > >
> > > > > -    route_table_run();
> > > > >      seq = route_table_get_change_seq();
> > > > >      if (rt_change_seqno != seq) {
> > > > >          rt_change_seqno = seq;
> > > > > @@ -386,7 +385,6 @@ netdev_vport_wait(void)
> > > > >  {
> > > > >      uint64_t seq;
> > > > >
> > > > > -    route_table_wait();
> > > > >      seq = route_table_get_change_seq();
> > > > >      if (rt_change_seqno != seq) {
> > > > >          poll_immediate_wake();
> > > > > diff --git a/lib/netdev.c b/lib/netdev.c
> > > > > index 3e50694..710739a 100644
> > > > > --- a/lib/netdev.c
> > > > > +++ b/lib/netdev.c
> > > > > @@ -45,6 +45,7 @@
> > > > >  #include "openflow/openflow.h"
> > > > >  #include "packets.h"
> > > > >  #include "poll-loop.h"
> > > > > +#include "route-table.h"
> > > > >  #include "seq.h"
> > > > >  #include "shash.h"
> > > > >  #include "smap.h"
> > > > > @@ -182,6 +183,7 @@ netdev_run(void)
> > > > >      struct netdev_registered_class *rc;
> > > > >
> > > > >      netdev_initialize();
> > > > > +    route_table_run();
> > > > >      ovs_mutex_lock(&netdev_class_mutex);
> > > > >      HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
> > > > >          if (rc->class->run) {
> > > > > @@ -201,6 +203,7 @@ netdev_wait(void)
> > > > >  {
> > > > >      struct netdev_registered_class *rc;
> > > > >
> > > > > +    route_table_wait();
> > > > >      ovs_mutex_lock(&netdev_class_mutex);
> > > > >      HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
> > > > >          if (rc->class->wait) {
> > > > >
> > >
>



More information about the dev mailing list