From: Alexander Smirnov <hidden> Date: 2012-07-02 06:22:43
Hello David,
I've received a notification from Fengguang Wu that there are new sparse
warnings are shown up after my patch sets.
Completely forgot about 'sparse', aghhr...
These patches fixes the following sparse warnings:
- drivers/ieee802154/at86rf230.c:610:2: warning:
'rc' may be used uninitialized in this function [-Wuninitialized]
- net/ieee802154/6lowpan.c:127:12: sparse:
symbol 'flist_lock' was not declared. Should it be static?
- net/mac802154/mac_cmd.c:58:17: warning:
symbol 'mac802154_get_phy' was not declared. Should it be static?
- net/mac802154/mib.c:42:23: warning:
symbol 'mac802154_slave_get_priv' was not declared. Should it be static?
With best regards,
Alexander Smirnov
Alexander Smirnov (2):
ieee802154: sparse warnings: make symbols static
drivers/ieee802154/at231rf230: remove unused return status
drivers/ieee802154/at86rf230.c | 3 +--
net/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
--
1.7.2.3
From: Alexander Smirnov <hidden> Date: 2012-07-02 06:22:45
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/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-07-02 06:37:54
On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
quoted hunk
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/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-07-02 06:44:43
On Mon, 2012-07-02 at 08:37 +0200, Eric Dumazet wrote:
On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
quoted
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/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Eric Dumazet <hidden> Date: 2012-07-02 06:49:14
On Mon, 2012-07-02 at 08:44 +0200, Eric Dumazet wrote:
On Mon, 2012-07-02 at 08:37 +0200, Eric Dumazet wrote:
quoted
On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
quoted
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/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
and of course commit 768f7c7c121e80f4 (6lowpan: add missing
spin_lock_init() ) must be reverted.
You should validate this code with LOCKDEP
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
del_timer() probably needs a del_timer_sync() too
From: Alexander Smirnov <hidden> Date: 2012-07-02 06:53:05
Dear Eric,
quoted
quoted
static DEFINE_SPINLOCK(flist_lock);
and of course commit 768f7c7c121e80f4 (6lowpan: add missing
spin_lock_init() ) must be reverted.
You should validate this code with LOCKDEP
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
del_timer() probably needs a del_timer_sync() too
From: Eric Dumazet <hidden> Date: 2012-07-02 07:09:04
On Mon, 2012-07-02 at 10:53 +0400, Alexander Smirnov wrote:
Dear Eric,
quoted
quoted
quoted
static DEFINE_SPINLOCK(flist_lock);
and of course commit 768f7c7c121e80f4 (6lowpan: add missing
spin_lock_init() ) must be reverted.
You should validate this code with LOCKDEP
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
del_timer() probably needs a del_timer_sync() too
Thanks a lot for the hints!
While you are changing this code, please add in
lowpan_alloc_new_frame() :
- 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...
- initialize frame->lock
From: Alexander Smirnov <hidden> Date: 2012-07-04 13:38:50
Hi Eric,
just a several questions:
quoted
quoted
static DEFINE_SPINLOCK(flist_lock);
and of course commit 768f7c7c121e80f4 (6lowpan: add missing
spin_lock_init() ) must be reverted.
Do I need to create 2 separate patches: one for revert and second to
initialize spinlock correctly, or I can combine these changes in one
patch?
You should validate this code with LOCKDEP
Nothing was shown by LOCKDEP for 6lowpan. :-(
I've selected the following options:
-*- Spinlock and rw-lock debugging: basic checks
-*- Mutex debugging: basic checks
-*- Lock debugging: detect incorrect freeing of live locks
[*] Lock usage statistics
[*] Lock dependency engine debugging
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
What would be the best way to solve this context mismatch? Can I do
something like following:
1. create some 6lowpan internal workqueue
2. replace lowpan_fragment_timer_expired() body by queue_work() with
current list_deleting routine
3. when 6lowpan is going to be deleted - I'll flush the queue and
remove all the timers and respective fragments
Alex
From: Eric Dumazet <hidden> Date: 2012-07-04 13:45:32
On Wed, 2012-07-04 at 17:38 +0400, Alexander Smirnov wrote:
Do I need to create 2 separate patches: one for revert and second to
initialize spinlock correctly, or I can combine these changes in one
patch?
you can combine patch
quoted
You should validate this code with LOCKDEP
Nothing was shown by LOCKDEP for 6lowpan. :-(
Because path was not hit ( fragment expire )
You would have to simulate a drop or something to trigger the lockdep
splat, when lowpan_fragment_timer_expired() fires.
I've selected the following options:
-*- Spinlock and rw-lock debugging: basic checks
-*- Mutex debugging: basic checks
-*- Lock debugging: detect incorrect freeing of live locks
[*] Lock usage statistics
[*] Lock dependency engine debugging
quoted
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
What would be the best way to solve this context mismatch? Can I do
something like following:
1. create some 6lowpan internal workqueue
2. replace lowpan_fragment_timer_expired() body by queue_work() with
current list_deleting routine
3. when 6lowpan is going to be deleted - I'll flush the queue and
remove all the timers and respective fragments
Just use the spin_lock_bh() variant to disable BH, so that timer doesnt
deadlock with you.