[ovs-dev] [PATCH] lacp: Really fix mutex initialization.
azhou at nicira.com
Wed May 7 21:11:23 UTC 2014
I assume you will back port this to branch-2.2
On Wed, May 7, 2014 at 1:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Wed, May 07, 2014 at 01:01:56PM -0700, Andy Zhou wrote:
>> On Wed, May 7, 2014 at 12:13 PM, Andy Zhou <azhou at nicira.com> wrote:
>> > I am looking at it.
>> > On Tue, May 6, 2014 at 12:50 PM, Ben Pfaff <blp at nicira.com> wrote:
>> >> Commit 2a3fb0aa3c (lacp: Don't lock potentially uninitialized mutex in
>> >> lacp_status().) fixed one bug related to acquiring the file scope 'mutex'
>> >> without initializing it. However, there was at least one other, in
>> >> lacp_unixctl_show(). One could just fix that one problem, but that leaves
>> >> the possibility that I might have missed one or two more. This commit
>> >> fixes the problem for good, by adding a helper that initializes the mutex
>> >> and then acquires it.
>> This makes the lacp locking somewhat different from other modules.
>> The logic looks fine.
>> Acked-by: Andy Zhou <azhou at nicira.com>
> I think it's OK for the locking to be a little different here.
>> >> It's not entirely clear why 'mutex' is a recursive mutex. I think that it
>> >> might be just because of the callback in lacp_run(). An alternate fix,
>> >> therefore, would be to eliminate the callback and therefore the need for
>> >> runtime initialization of the mutex.
>> It seems when we send a lacp packet out while holding the mutex,
>> compose_output_action__ could
>> send it right back lacp via process_special(). May be we can queue up
>> the packets in lacp_run,
>> and call send_pdu outside of lock?
> Yes, that's what I concluded too.
> I'll push this in a minute.
More information about the dev