On Wed, Dec 14, 2011 at 4:37 PM, Jesse Gross <je...@nicira.com> wrote:
> On Wed, Dec 14, 2011 at 3:47 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>> On Wed, Dec 14, 2011 at 3:34 PM, Jesse Gross <je...@nicira.com> wrote:
>>> On Wed, Dec 14, 2011 at 3:07 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>>>> On Wed, Dec 14, 2011 at 2:47 PM, Ansis Atteka <aatt...@nicira.com> wrote:
>>>>> On Wed, Dec 14, 2011 at 2:08 PM, Pravin B Shelar <pshe...@nicira.com>
>>>>> wrote:
>>>>>> +int genl_exec(genl_exec_func_t func, void *data)
>>>>>> +{
>>>>>> + struct sk_buff *skb;
>>>>>> +
>>>>>> + skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>>>>> + if (!skb)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + genlmsg_put(skb, 0, 0, &genl_exec_family, NLM_F_REQUEST,
>>>>>> GENL_EXEC_RUN);
>>>>>> +
>>>>>> + mutex_lock(&genl_exec_lock);
>>>>>> + genl_exec_function = func;
>>>>>> + genl_exec_data = data;
>>>>>> + init_completion(&done);
>>>>>> + genlmsg_unicast(&init_net, skb, 0);
>>>>>> + wait_for_completion(&done);
>>>>>> + genl_exec_function = NULL;
>>>>>
>>>>> Shouldn't you prepare here a copy of genl_exec_function_ret before
>>>>> actually
>>>>> returning it? I sense a small-probability of a race condition here.
>>>>>>
>>>> right, I need to do it inside lock. Even with lock it is not
>>>> completely safe as callback is not taking any lock. But I think that
>>>> is not issue for now.
>>>
>>> Isn't all of this executing synchronously anyways, so the callback
>>> actually is protected by genl_exec_lock and the completion stuff isn't
>>> necessary?
>> Its not same across kernel that we care.
>> callbacks are executed synchronously from 2.6.24 onward.
>
> Are you sure? I think it just takes a slightly more convoluted path
> to get there. If doesn't execute in this context, what context does
> it execute in?
>
> On 2.6.18, for example, I think this is the call stack:
> - genl_exec
> - genlmsg_unicast
> - nlmsg_unicast
> - netlink_unicast
> - netlink_sendskb (skb gets enqueued to socket buffer)
> - netlink_data_ready AKA sk->sk_data_ready
> - genl_rcv AKA nlk->data_ready
> - netlink_run_queue (skb gets dequeued from socket buffer)
> - netlink_rcv_skb
> - genl_rcv_mesg
> - genl_exec_cmd AKA doit
>
> So there's a lot of useless enqueueing, dequeueing, waking up
> processes, etc. that are really only needed when sending data to
> userspace. That was shortcut in later kernels but it doesn't really
> change anything.
right, but there is genl_trylock() called in genl_rcv().
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
