On 05.12.2010 18:27, Marc Kleine-Budde wrote:
> On 12/05/2010 05:45 PM, Oliver Hartkopp wrote:
>> On 05.12.2010 16:15, Marc Kleine-Budde wrote:
>>> On 12/05/2010 10:54 AM, Michal Sojka wrote:
>>
>>>> diff --git a/net/can/gw.c b/net/can/gw.c
>>>> index 94ba3f1..7779ca6 100644
>>>> --- a/net/can/gw.c
>>>> +++ b/net/can/gw.c
>>>> @@ -822,11 +822,14 @@ static int cgw_create_job(struct sk_buff *skb,  
>>>> struct nlmsghdr *nlh,
>>>>         if (gwj->dst.dev->type != ARPHRD_CAN)
>>>>                 goto put_src_dst_out;
>>>>                 
>>>> -       spin_lock(&cgw_list_lock);
>>>>  
>>>>         err = cgw_register_filter(gwj);
>>>> -       if (!err)
>>>> -               hlist_add_head_rcu(&gwj->list, &cgw_list);
>>>> +       if (err)
>>>> +               goto put_src_dst_out;
>>>> +
>>>> +       spin_lock(&cgw_list_lock);
>>>> +
>>>> +       hlist_add_head_rcu(&gwj->list, &cgw_list);
>>>>  
>>>>         spin_unlock(&cgw_list_lock);
>>>
>>> The fix looks good!
>>
>> At least it fixes the issue, i had in my kernel log ... (in my other post).
>>
>> My problem is, that the notifier in cgw_notifier() expects the registered CAN
>> filter and the hlist entry to be available together.
>>
>> That was the reason, why it put this together in the locked region.
> 
> then we must use the 2nd fix
> 
>> diff --git a/net/can/af_can.c b/net/can/af_can.c
>> index 702be5a..b046ff0 100644
>> --- a/net/can/af_can.c
>> +++ b/net/can/af_can.c
>> @@ -418,7 +418,7 @@ int can_rx_register(struct net_device *dev, canid_t 
>> can_id, canid_t mask,
>>         if (dev && dev->type != ARPHRD_CAN)
>>                 return -ENODEV;
>>  
>> -       r = kmem_cache_alloc(rcv_cache, GFP_KERNEL);
>> +       r = kmem_cache_alloc(rcv_cache, GFP_ATOMIC);
>>         if (!r)
>>                 return -ENOMEM;
>>  
> 

Hm - in general there's no reason to have this kmem_cache_alloc() running with
GFP_ATOMIC as can_rx_register() is always triggered from userspace- or from
softirq-context (notifier).

I expect the problem somewhere in the netlink/locking stuff that has changed
some time ago. I'll take a look on that (tomorrow?).

Regards,
Oliver
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users

Reply via email to