On 9/2/14, 1:39 PM, "Pravin Shelar" <pshe...@nicira.com> wrote:
>On Fri, Aug 29, 2014 at 4:52 PM, Daniele Di Proietto ><ddiproie...@vmware.com> wrote: >> dp_netdev_free() must free 'dp->upcall_rwlock', but when upcalls are >>disabled >> (if the datapath is being freed upcalls should be disabled) >>'dp->upcall_rwlock' >> is taken and freeing it causes an assertion to fail. >> >> This commit takes makes sure that the upcalls are disabled and releases >> 'dp->upcall_rwlock' before freeing it. A simple testcase is added to >>detect the >> failure. >> --- >> lib/dpif-netdev.c | 8 ++++++++ >> tests/dpif-netdev.at | 8 ++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 3d09326..6db325f 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -513,6 +513,7 @@ dpif_netdev_open(const struct dpif_class *class, >>const char *name, >> static void >> dp_netdev_free(struct dp_netdev *dp) >> OVS_REQUIRES(dp_netdev_mutex) >> + OVS_NO_THREAD_SAFETY_ANALYSIS >I am not sure about these two annotations, how does it work in this case? If I understand your concern properly, the two annotations work independently, i.e. (at least on my system, with clang 3.4) the compiler still complains if I try to call dp_netdev_free() without holding the Œdp_netdev_mutex¹, but it doesn¹t try to analyze what¹s inside the function. I do not particularly like having to add OVS_NO_THREAD_SAFETY_ANALYSIS annotations, but, given the way we use Œupcall_rwlock¹, I do not always find a way to avoid it. Would you prefer having a separate Œupcall_rwlock_destroy()¹ function (marked with OVS_NO_THREAD_SAFETY_ANALYSIS), perhaps? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
