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 <[email protected]> >>>> --- >>>> 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.
