From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:35
Dear David, Eric,
this patch-set is mostly intended to fix sparse and LOCKDEP warnings.
It contains some my previous patches reworked and extended according
to the hints from Eric Dumazet and Fengguang Wu. Many thanks to they!
With best regards,
Alex
Alexander Smirnov (6):
6lowpan: revert: add missing spin_lock_init()
drivers/ieee802154/at86rf230: rework irq handler
mac802154: add get short address method
6lowpan: rework fragment-deleting routine
6lowpan: get extra headroom in allocated frame
mac802154: sparse warnings: make symbols static
drivers/ieee802154/at86rf230.c | 14 ++++++--------
net/ieee802154/6lowpan.c | 36 +++++++++++++++++++-----------------
net/mac802154/mac802154.h | 1 +
net/mac802154/mac_cmd.c | 4 +++-
net/mac802154/mib.c | 16 +++++++++++++++-
5 files changed, 44 insertions(+), 27 deletions(-)
--
1.7.2.3
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:36
Revert the commit 768f7c7c121e80f458a9d013b2e8b169e5dfb1e5 to initialize
spinlock in the more preferable way and make it static to avoid sparse
warning.
Signed-off-by: Alexander Smirnov <redacted>
---
net/ieee802154/6lowpan.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:38
Fix LOCKDEP bug message for the irq handler spinlock.
Make the irq processing code more explicit and stable.
Signed-off-by: Alexander Smirnov <redacted>
---
drivers/ieee802154/at86rf230.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:40
Add method to get the device short 802.15.4 address. This call
needed by ieee802154 layer to satisfy 'iz list' request from
the user space.
Signed-off-by: Alexander Smirnov <redacted>
---
net/mac802154/mac802154.h | 1 +
net/mac802154/mac_cmd.c | 2 ++
net/mac802154/mib.c | 14 ++++++++++++++
3 files changed, 17 insertions(+), 0 deletions(-)
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:42
6lowpan module starts collecting incomming frames and fragments
right after lowpan_module_init() therefor it will be better to
clean unfinished fragments in lowpan_cleanup_module() function
instead of doing it when link goes down.
Changed spinlocks type to prevent deadlock with expired timer event
and removed unused one.
Signed-off-by: Alexander Smirnov <redacted>
---
net/ieee802154/6lowpan.c | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
@@ -113,7 +113,6 @@ struct lowpan_dev_record {structlowpan_fragment{structsk_buff*skb;/* skb to be assembled */-spinlock_tlock;/* concurency lock */u16length;/* length to be assemled */u32bytes_rcv;/* bytes received */u16tag;/* current fragment tag */
@@ -761,7 +760,7 @@ lowpan_process_data(struct sk_buff *skb)if((frame->bytes_rcv==frame->length)&&frame->timer.expires>jiffies){/* if timer haven't expired - first of all delete it */-del_timer(&frame->timer);+del_timer_sync(&frame->timer);list_del(&frame->list);spin_unlock(&flist_lock);
@@ -1264,9 +1253,24 @@ out:staticvoid__exitlowpan_cleanup_module(void){+structlowpan_fragment*frame,*tframe;+lowpan_netlink_fini();dev_remove_pack(&lowpan_packet_type);++/* Now 6lowpan packet_type is removed, so no new fragments are+*expectedonRX,thereforethat'sthetimetocleanincomplete+*fragments.+*/+spin_lock_bh(&flist_lock);+list_for_each_entry_safe(frame,tframe,&lowpan_fragments,list){+del_timer_sync(&frame->timer);+list_del(&frame->list);+dev_kfree_skb(frame->skb);+kfree(frame);+}+spin_unlock_bh(&flist_lock);}module_init(lowpan_init_module);
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:44
Use netdev_alloc_skb_ip_align() instead of alloc_skb() to get some
extra headroom in case we need to forward this frame in a tunnel or
something else.
Signed-off-by: Alexander Smirnov <redacted>
---
net/ieee802154/6lowpan.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
From: Alexander Smirnov <hidden> Date: 2012-07-09 10:26:46
Make symbols static to avoid the following warning shown up
by sparse:
warning: symbol ... was not declared. Should it be static?
Signed-off-by: Alexander Smirnov <redacted>
---
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-07-09 10:53:41
On Mon, 2012-07-09 at 14:22 +0400, Alexander Smirnov wrote:
quoted hunk
6lowpan module starts collecting incomming frames and fragments
right after lowpan_module_init() therefor it will be better to
clean unfinished fragments in lowpan_cleanup_module() function
instead of doing it when link goes down.
Changed spinlocks type to prevent deadlock with expired timer event
and removed unused one.
Signed-off-by: Alexander Smirnov <redacted>
---
net/ieee802154/6lowpan.c | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
@@ -113,7 +113,6 @@ struct lowpan_dev_record {structlowpan_fragment{structsk_buff*skb;/* skb to be assembled */-spinlock_tlock;/* concurency lock */u16length;/* length to be assemled */u32bytes_rcv;/* bytes received */u16tag;/* current fragment tag */
@@ -761,7 +760,7 @@ lowpan_process_data(struct sk_buff *skb)if((frame->bytes_rcv==frame->length)&&frame->timer.expires>jiffies){/* if timer haven't expired - first of all delete it */-del_timer(&frame->timer);+del_timer_sync(&frame->timer);list_del(&frame->list);spin_unlock(&flist_lock);
@@ -1264,9 +1253,24 @@ out:staticvoid__exitlowpan_cleanup_module(void){+structlowpan_fragment*frame,*tframe;+lowpan_netlink_fini();dev_remove_pack(&lowpan_packet_type);++/* Now 6lowpan packet_type is removed, so no new fragments are+*expectedonRX,thereforethat'sthetimetocleanincomplete+*fragments.+*/+spin_lock_bh(&flist_lock);+list_for_each_entry_safe(frame,tframe,&lowpan_fragments,list){+del_timer_sync(&frame->timer);+list_del(&frame->list);+dev_kfree_skb(frame->skb);+kfree(frame);+}+spin_unlock_bh(&flist_lock);}module_init(lowpan_init_module);
Problem is lowpan_fragment_timer_expired() can race with this code.
del_timer_sync() might block here if lowpan_fragment_timer_expired() is
waiting/spinning for spin_lock(&flist_lock)
You cant call del_timer_sync() holding flist_lock, you should find
another way to solve the problem.
Its explained in kernel/timer.c :
#ifdef CONFIG_SMP
/**
* del_timer_sync - deactivate a timer and wait for the handler to finish.
* @timer: the timer to be deactivated
*
* This function only differs from del_timer() on SMP: besides deactivating
* the timer it also makes sure the handler has finished executing on other
* CPUs.
*
* Synchronization rules: Callers must prevent restarting of the timer,
* otherwise this function is meaningless. It must not be called from
* interrupt contexts. The caller must not hold locks which would prevent
* completion of the timer's handler. The timer's handler must not call
* add_timer_on(). Upon exit the timer is not queued and the handler is
* not running on any CPU.
*
* Note: You must not hold locks that are held in interrupt context
* while calling this function. Even if the lock has nothing to do
* with the timer in question. Here's why:
*
* CPU0 CPU1
* ---- ----
* <SOFTIRQ>
* call_timer_fn();
* base->running_timer = mytimer;
* spin_lock_irq(somelock);
* <IRQ>
* spin_lock(somelock);
* del_timer_sync(mytimer);
* while (base->running_timer == mytimer);
*
* Now del_timer_sync() will never return and never release somelock.
* The interrupt on the other CPU is waiting to grab somelock but
* it has interrupted the softirq that CPU0 is waiting to finish.
*
* The function returns whether it has deactivated a pending timer or not.
*/