[ovs-dev] [PATCH V2 01/11] clang: Add annotations for thread safety check.

[Alex Wang]

Thanks Ben for your review, very glad to see this applied.

On Tue, Jul 30, 2013 at 9:40 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, Jul 30, 2013 at 03:31:48PM -0700, Alex Wang wrote:
> > From: Ethan Jackson <ethan at nicira.com>
> >
> > This commit adds annotations for thread safety check. And the
> > check can be conducted by using -Wthread-safety flag in clang.
> >
> > Co-authored-by: Alex Wang <alexw at nicira.com>
> > Signed-off-by: Alex Wang <alexw at nicira.com>
> > Signed-off-by: Ethan Jackson <ethan at nicira.com>
>
> The lockfile logging change in this patch caused test failures for me.
> I didn't see a reason for the logging change, so I just dropped it.  We
> can do it in a separate patch if there is a good reason..
>


This actually relates to the unresolved comment. I misunderstood.
I thought you want to add error message to where EDEADLK is returned.
And that's why I added VLOG_ERR and modified the lockfile.at.

So, I'm still not very clear. Do you suggest that we should show 'where' in
"ovs_abort()" in lib/ovs-thread.c? Could you give an example of using the
'where'?


> I did a little editing of compiler and ovs-thread.[ch].  Diff appended.
> Hope it's OK.
>

Thanks for the refinement of compiler.h. Even though I saw that way of
commenting
several times, I still couldn't take advantage of it. More to learn and to
really do it myself. ;D

Also, thanks for the cleanup of ovs-thread.[ch]


> My only unresolved comment is that the 'where' member of the new struct
> is write-only: nothing ever reads it.  But this is important enough that
> I'm willing to resolve that one way or another later.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130730/175a74d3/attachment-0003.html>