[ovs-dev] [PATCH v3] dpif-netdev: Polling threads directly call ofproto upcall functions.

Ben Pfaff blp at nicira.com
Tue Jun 24 21:44:38 UTC 2014


I think you mean Clang rather than sparse in this case.

Let's go with your original solution.  It sounds like it's getting
complicated.  I originally assumed that you just overlooked something.

On Tue, Jun 24, 2014 at 09:38:31PM +0000, Ryan Wilson 76511 wrote:
> So I added OVS_NO_THREAD_SAFETY_ANALYSIS to create_dp_netdev() and removed
> OVS_NO_THREAD_SAFETY_ANALYSIS annotations on dpif_netdev_disable_upcall()
> and dpif_netdev_enable_upcall(). However, I still get a warning from
> sparse about dp->upcall_rwlock being locked / unlocked at the end of
> dpif_netdev_*_upcall(). Problem is that both take in dpif as arguments,
> not dp, so adding lock annotations is not a possibility here.
> 
> I can mark create_dp_netdev() as OVS_NO_THREAD_SAFETY_ANALYSIS as well as
> dpif_netdev_*_upcall(), but that seems redundant to me. Let me know what
> you think, maybe there's an obvious solution here I'm not seeing.
> 
> Ryan
> 
> On 6/24/14 2:14 PM, "Ben Pfaff" <blp at nicira.com> wrote:
> 
> >On Tue, Jun 24, 2014 at 01:41:13PM -0700, Ryan Wilson wrote:
> >> On Tue, Jun 24, 2014 at 8:44 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > I'm surprised that dpif_netdev_disable_upcall() is marked
> >> > OVS_NO_THREAD_SAFETY_ANALYSIS instead of
> >> > OVS_ACQUIRES(dp->upcall_rwlock).  Similarly for
> >> > dpif_netdev_enable_upcall().
> >> >
> >> >
> >> Its marked this way because when creating a dp_netdev, I must initialize
> >> and lock the dp->upcall_rwlock. If I don't add
> >> OVS_NO_THREAD_SAFETY_ANALYSIS, sparse complains dp->upcall_rwlock is
> >>locked
> >> at the end of the function. Unfortunately, I can't use
> >> OVS_ACQUIRES(dp->upcall_rwlock) since create_dp_netdev() creates the
> >> dp_netdev and does not take dp as an argument. Is there another way we
> >>deal
> >> with issues like this?
> >
> >It seems to me that create_dp_netdev(), which initializes and takes
> >the rwlock, is the problematic function, so I'd be inclined to mark
> >that one as OVS_NO_THREAD_SAFETY_ANALYSIS.
> >_______________________________________________
> >dev mailing list
> >dev at openvswitch.org
> >https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/
> >listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%
> >3D%3D%0A&m=l0q7tyrVOdg7oAIBQE7VHqBq0e25NDCtaUNfHpbgF4g%3D%0A&s=db3e738004f
> >ecfe62c1aa0cd2dfe1cc8fd6f65f360ec404a036878aceedca346
> 



More information about the dev mailing list