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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
