[PATCH 06/12] net/rtdev: ensure per-device skbs get mapped at registration

Philippe Gerum rpm at xenomai.org
Wed Feb 6 10:47:20 CET 2019


On 2/6/19 10:08 AM, Jan Kiszka wrote:
> On 06.02.19 10:02, Philippe Gerum wrote:
>> On 1/24/19 7:24 PM, Jan Kiszka wrote:
>>> On 24.01.19 16:34, Philippe Gerum wrote:
>>>> This patch works around a regression introduced by #74464ee37d0,
>>>> causing a new device's skbs not to be passed to its ->map_rtskb()
>>>> handler when registered, breaking further DMA inits in the driver.
>>>>
>>>> Signed-off-by: Philippe Gerum <rpm at xenomai.org>
>>>> ---
>>>>    kernel/drivers/net/stack/rtdev.c | 37
>>>> +++++++++++++++++++++++---------
>>>>    1 file changed, 27 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/kernel/drivers/net/stack/rtdev.c
>>>> b/kernel/drivers/net/stack/rtdev.c
>>>> index 631ca1804..286d08cb1 100644
>>>> --- a/kernel/drivers/net/stack/rtdev.c
>>>> +++ b/kernel/drivers/net/stack/rtdev.c
>>>> @@ -45,6 +45,7 @@ struct rtnet_device
>>>> *rtnet_devices[MAX_RT_DEVICES];
>>>>    static struct rtnet_device  *loopback_device;
>>>>    static DEFINE_RTDM_LOCK(rtnet_devices_rt_lock);
>>>>    static LIST_HEAD(rtskb_mapped_list);
>>>> +static LIST_HEAD(rtskb_mapwait_list);
>>>>      LIST_HEAD(event_hook_list);
>>>>    DEFINE_MUTEX(rtnet_devices_nrt_lock);
>>>> @@ -459,8 +460,12 @@ int rtdev_map_rtskb(struct rtskb *skb)
>>>>        }
>>>>        }
>>>>    -    if (!err && skb->buf_dma_addr != RTSKB_UNMAPPED)
>>>> -    list_add(&skb->entry, &rtskb_mapped_list);
>>>> +    if (!err) {
>>>> +        if (skb->buf_dma_addr != RTSKB_UNMAPPED)
>>>> +            list_add(&skb->entry, &rtskb_mapped_list);
>>>> +        else
>>>> +            list_add(&skb->entry, &rtskb_mapwait_list);
>>>> +    }
>>>>          mutex_unlock(&rtnet_devices_nrt_lock);
>>>>    @@ -471,19 +476,31 @@ int rtdev_map_rtskb(struct rtskb *skb)
>>>>      static int rtdev_map_all_rtskbs(struct rtnet_device *rtdev)
>>>>    {
>>>> -    struct rtskb *skb;
>>>> -    int err = 0;
>>>> +    struct rtskb *skb, *n;
>>>> +    int err;
>>>>          if (!rtdev->map_rtskb)
>>>> -    return 0;
>>>> +        return 0;
>>>>    -    list_for_each_entry(skb, &rtskb_mapped_list, entry) {
>>>> -    err = rtskb_map(rtdev, skb);
>>>> -    if (err)
>>>> -       break;
>>>> +    if (!list_empty(&rtskb_mapped_list)) {
>>>
>>> Why this extra check? list_for_each_entry should just do nothing if the
>>> list is empty.
>>>
>>>> +        list_for_each_entry(skb, &rtskb_mapped_list, entry) {
>>>> +            err = rtskb_map(rtdev, skb);
>>>> +            if (err)
>>>> +                return err;
>>>> +        }
>>>>        }
>>>>    -    return err;
>>>> +    if (!list_empty(&rtskb_mapwait_list)) {
>>>
>>> Same here.
>>>
>>>> +        list_for_each_entry_safe(skb, n, &rtskb_mapwait_list, entry) {
>>>> +            err = rtskb_map(rtdev, skb);
>>>> +            if (err)
>>>> +                return err;
>>>> +            list_del(&skb->entry);
>>>> +            list_add(&skb->entry, &rtskb_mapped_list);
>>>> +        }
>>>> +    }
>>>> +
>>>> +    return 0;
>>>>    }
>>>>     
>>>
>>> Style mix: Eventually, we should switch all RTnet code to kernel style.
>>> For now, we have 4-space-indention, and we should keep it until then.
>>> Mixing things will only make it more messy.
>>>
>>
>> There is a massive commit [1] pending in my queue fixing RTnet wrt
>> coding style, which I'm not going to paste here. This is the result of
>> filtering the code through scripts/Lindent basically.
>>
>> Would you agree on merging such kind of commit into RTnet? If so, the
>> vlan and multicast support I'm currently adapting from Gilles' past work
>> should go on top of such changes.
> 
> I would take such cleanup. It's a timing issue, though, because any
> further stable fixes that come after that may be harder to backport. New
> feature are fine afterwards, of course.
> 

Gilles' work I was referring to are unlikely to go to -stable given the
scope of the changes involved, so I'll target -next instead for this,
based on the reformatting patch.

-- 
Philippe.



More information about the Xenomai mailing list