2020-04-11 00:21:10

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 0/9] Lock warning cleanups

This patch series adds missing annotations to various functions,
that register warnings of context imbalance when built with Sparse tool.
The adds fix the warnings, improve on readability of the code
and give better insight or directive on what the functions are actually
doing.

Jules Irenge (9):
dm snapshot: Add missing annotation for dm_exception_table_lock() and
dm_exception_table_unlock()
mt76: remove unnecessary annotations
hostap: Add missing annotations for prism2_bss_list_proc_start() and
prism2_bss_list_proc_stop
mac80211: Add missing annotation for brcms_rfkill_set_hw_state()
mac80211: Add missing annotation for brcms_down()
scsi: libsas: Add missing annotation for sas_ata_qc_issue()
scsi: bnx2fc: Add missing annotation for bnx2fc_abts_cleanup()
power: wakeup: Add missing annotation for
wakeup_sources_stats_seq_start() and wakeup_sources_stats_seq_stop()
power: wakeup: Add missing annotation for wakeup_sources_read_lock()
and wakeup_sources_read_unlock()

drivers/base/power/wakeup.c | 4 ++++
drivers/md/dm-snap.c | 4 ++++
.../net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c | 2 ++
drivers/net/wireless/intersil/hostap/hostap_proc.c | 2 ++
drivers/net/wireless/mediatek/mt76/tx.c | 4 +---
drivers/scsi/bnx2fc/bnx2fc_io.c | 1 +
drivers/scsi/libsas/sas_ata.c | 1 +
7 files changed, 15 insertions(+), 3 deletions(-)

--
2.24.1


2020-04-11 00:21:10

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 1/9] dm snapshot: Add missing annotation for dm_exception_table_lock() and dm_exception_table_unlock()

Sparse reports warnings at dm_exception_table_lock()
and dm_exception_table_unlock()

warning: context imbalance in dm_exception_table_lock()
- wrong count at exit
warning: context imbalance in dm_exception_table_unlock()
- unexpected unlock

The root cause is the missing annotation at dm_exception_table_lock()
and dm_exception_table_unlock()

Add the missing __acquires(lock->complete_slot) annotation
Add the missing __acquires(lock->pending_slot) annotation
Add the missing __releases(lock->pending_slot) annotation
Add the missing __releases(lock->complete_slot) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/md/dm-snap.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 6b11a266299f..1831dd28de5c 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -640,12 +640,16 @@ static void dm_exception_table_lock_init(struct dm_snapshot *s, chunk_t chunk,
}

static void dm_exception_table_lock(struct dm_exception_table_lock *lock)
+ __acquires(lock->complete_slot)
+ __acquires(lock->pending_slot)
{
hlist_bl_lock(lock->complete_slot);
hlist_bl_lock(lock->pending_slot);
}

static void dm_exception_table_unlock(struct dm_exception_table_lock *lock)
+ __releases(lock->pending_slot)
+ __releases(lock->complete_slot)
{
hlist_bl_unlock(lock->pending_slot);
hlist_bl_unlock(lock->complete_slot);
--
2.24.1

2020-04-11 00:21:11

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 2/9] mt76: remove unnecessary annotations

Sparse report warnings at mt76_tx_status_unlock() and mt76_tx_status_lock()

warning: context imbalance in mt76_tx_status_lock() - wrong count at exit
warning: context imbalance in mt76_tx_status_unlock() - unexpected unlock

The root cause is the additional __acquire(&dev->status_list.lock)
and __release(&dev->status_list.unlock) called
from inside mt76_tx_status_lock() and mt76_tx_status_unlock().

Remove __acquire(&dev->status_list.lock) annotation
Remove __releases(&dev->status_list.unlock)
Correct &dev->status_list.unlock to &dev->status_list.lock
-unlock not defined in the sk_buff_head struct

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/net/wireless/mediatek/mt76/tx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
index 7ee91d946882..7581ba9c2e95 100644
--- a/drivers/net/wireless/mediatek/mt76/tx.c
+++ b/drivers/net/wireless/mediatek/mt76/tx.c
@@ -101,18 +101,16 @@ mt76_tx_status_lock(struct mt76_dev *dev, struct sk_buff_head *list)
{
__skb_queue_head_init(list);
spin_lock_bh(&dev->status_list.lock);
- __acquire(&dev->status_list.lock);
}
EXPORT_SYMBOL_GPL(mt76_tx_status_lock);

void
mt76_tx_status_unlock(struct mt76_dev *dev, struct sk_buff_head *list)
- __releases(&dev->status_list.unlock)
+ __releases(&dev->status_list.lock)
{
struct sk_buff *skb;

spin_unlock_bh(&dev->status_list.lock);
- __release(&dev->status_list.unlock);

while ((skb = __skb_dequeue(list)) != NULL)
ieee80211_tx_status(dev->hw, skb);
--
2.24.1

2020-04-11 00:21:26

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 3/9] hostap: Add missing annotations for prism2_bss_list_proc_start() and prism2_bss_list_proc_stop

Sparse reports warnings at prism2_bss_list_proc_start() and prism2_bss_list_proc_stop()

warning: context imbalance in prism2_wds_proc_stop() - unexpected unlock
warning: context imbalance in prism2_bss_list_proc_start() - wrong count at exit

The root cause is the missing annotations at prism2_bss_list_proc_start()

Add the missing __acquires(&local->lock) annotation
Add the missing __releases(&local->lock) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/net/wireless/intersil/hostap/hostap_proc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intersil/hostap/hostap_proc.c b/drivers/net/wireless/intersil/hostap/hostap_proc.c
index a2ee4693eaed..97c270845fd1 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_proc.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_proc.c
@@ -149,6 +149,7 @@ static int prism2_bss_list_proc_show(struct seq_file *m, void *v)
}

static void *prism2_bss_list_proc_start(struct seq_file *m, loff_t *_pos)
+ __acquires(&local->lock)
{
local_info_t *local = PDE_DATA(file_inode(m->file));
spin_lock_bh(&local->lock);
@@ -162,6 +163,7 @@ static void *prism2_bss_list_proc_next(struct seq_file *m, void *v, loff_t *_pos
}

static void prism2_bss_list_proc_stop(struct seq_file *m, void *v)
+ __releases(&local->lock)
{
local_info_t *local = PDE_DATA(file_inode(m->file));
spin_unlock_bh(&local->lock);
--
2.24.1

2020-04-11 00:21:30

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 7/9] scsi: bnx2fc: Add missing annotation for bnx2fc_abts_cleanup()

Sparse reports a warning at bnx2fc_abts_cleanup()

warning: context imbalance in bnx2fc_abts_cleanup() - unexpected unlock

The root cause is the missing annotation at bnx2fc_abts_cleanup()

Add the missing __must_hold(&tgt->tgt_lock) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 4c8122a82322..b45f40db9379 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1081,6 +1081,7 @@ int bnx2fc_eh_device_reset(struct scsi_cmnd *sc_cmd)
}

static int bnx2fc_abts_cleanup(struct bnx2fc_cmd *io_req)
+ __must_hold(&tgt->tgt_lock)
{
struct bnx2fc_rport *tgt = io_req->tgt;
unsigned int time_left;
--
2.24.1

2020-04-11 00:21:34

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 8/9] power: wakeup: Add missing annotation for wakeup_sources_stats_seq_start() and wakeup_sources_stats_seq_stop()

Sparse reports warnings at wakeup_sources_stats_seq_start()
and wakeup_sources_stats_seq_stop()

warning: context imbalance in wakeup_sources_stats_seq_start()
- wrong count at exit
context imbalance in wakeup_sources_stats_seq_stop()
- unexpected unlock

The root cause is the missing annotation at
wakeup_sources_stats_seq_start() and wakeup_sources_stats_seq_stop()

Add the missing __acquires(&wakeup_srcu) annotation
Add the missing __releases(&wakeup_srcu) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/base/power/wakeup.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 27f3e60608e5..41ce086d8f57 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -1092,6 +1092,7 @@ static int print_wakeup_source_stats(struct seq_file *m,

static void *wakeup_sources_stats_seq_start(struct seq_file *m,
loff_t *pos)
+ __acquires(&wakeup_srcu)
{
struct wakeup_source *ws;
loff_t n = *pos;
@@ -1132,6 +1133,7 @@ static void *wakeup_sources_stats_seq_next(struct seq_file *m,
}

static void wakeup_sources_stats_seq_stop(struct seq_file *m, void *v)
+ __releases(&wakeup_srcu)
{
int *srcuidx = m->private;

--
2.24.1

2020-04-11 00:21:44

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 5/9] mac80211: Add missing annotation for brcms_down()

Sparse reports a warning at brcms_down()

warning: context imbalance in brcms_down()
- unexpected unlock
The root cause is the missing annotation at brcms_down()
Add the missing __must_hold(&wl->lock) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
index c3dbeacea6ca..648efcbc819f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
@@ -1431,6 +1431,7 @@ int brcms_up(struct brcms_info *wl)
* precondition: perimeter lock has been acquired
*/
void brcms_down(struct brcms_info *wl)
+ __must_hold(&wl->lock)
{
uint callbacks, ret_val = 0;

--
2.24.1

2020-04-11 00:22:07

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 6/9] scsi: libsas: Add missing annotation for sas_ata_qc_issue()

Sparse reports a warning at sas_ata_qc_issue()

warning: context imbalance in sas_ata_qc_issue() - unexpected unlock
The root cause is the missing annotation at sas_ata_qc_issue()

Add the missing __must_hold(ap->lock) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/scsi/libsas/sas_ata.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index c5a828a041e0..5d716d388707 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -160,6 +160,7 @@ static void sas_ata_task_done(struct sas_task *task)
}

static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
+ __must_hold(ap->lock)
{
struct sas_task *task;
struct scatterlist *sg;
--
2.24.1

2020-04-11 00:22:59

by Jules Irenge

[permalink] [raw]
Subject: [PATCH 9/9] power: wakeup: Add missing annotation for wakeup_sources_read_lock() and wakeup_sources_read_unlock()

Sparse reports warnings at wakeup_sources_read_lock()
and wakeup_sources_read_unlock()

warning: context imbalance in wakeup_sources_read_lock()
- wrong count at exit
context imbalance in wakeup_sources_read_unlock()
- unexpected unlock

The root cause is the missing annotation at
wakeup_sources_read_lock() and wakeup_sources_read_unlock()

Add the missing __acquires(&wakeup_srcu) annotation
Add the missing __releases(&wakeup_srcu) annotation

Signed-off-by: Jules Irenge <[email protected]>
---
drivers/base/power/wakeup.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 41ce086d8f57..753e9a46e04e 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -254,6 +254,7 @@ EXPORT_SYMBOL_GPL(wakeup_source_unregister);
* This index must be passed to the matching wakeup_sources_read_unlock().
*/
int wakeup_sources_read_lock(void)
+ __acquires(&wakeup_srcu)
{
return srcu_read_lock(&wakeup_srcu);
}
@@ -264,6 +265,7 @@ EXPORT_SYMBOL_GPL(wakeup_sources_read_lock);
* @idx: return value from corresponding wakeup_sources_read_lock()
*/
void wakeup_sources_read_unlock(int idx)
+ __releases(&wakeup_srcu)
{
srcu_read_unlock(&wakeup_srcu, idx);
}
--
2.24.1

2020-04-15 14:47:46

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 6/9] scsi: libsas: Add missing annotation for sas_ata_qc_issue()


Jules,

> Sparse reports a warning at sas_ata_qc_issue()
>
> warning: context imbalance in sas_ata_qc_issue() - unexpected unlock
> The root cause is the missing annotation at sas_ata_qc_issue()
>
> Add the missing __must_hold(ap->lock) annotation

Applied to 5.8/scsi-queue, thanks!

--
Martin K. Petersen Oracle Linux Engineering

2020-04-15 19:47:55

by John Garry

[permalink] [raw]
Subject: Re: [PATCH 6/9] scsi: libsas: Add missing annotation for sas_ata_qc_issue()

On 11/04/2020 01:19, Jules Irenge wrote:
> Sparse reports a warning at sas_ata_qc_issue()
>
> warning: context imbalance in sas_ata_qc_issue() - unexpected unlock
> The root cause is the missing annotation at sas_ata_qc_issue()
>
> Add the missing __must_hold(ap->lock) annotation
>
> Signed-off-by: Jules Irenge <[email protected]>

that looks ok...

Reviewed-by: John Garry <[email protected]>

> ---
> drivers/scsi/libsas/sas_ata.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index c5a828a041e0..5d716d388707 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -160,6 +160,7 @@ static void sas_ata_task_done(struct sas_task *task)
> }
>
> static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
> + __must_hold(ap->lock)
> {
> struct sas_task *task;
> struct scatterlist *sg;
>

2020-04-15 22:39:45

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/9] hostap: Add missing annotations for prism2_bss_list_proc_start() and prism2_bss_list_proc_stop

Jules Irenge <[email protected]> wrote:

> Sparse reports warnings at prism2_bss_list_proc_start() and prism2_bss_list_proc_stop()
>
> warning: context imbalance in prism2_wds_proc_stop() - unexpected unlock
> warning: context imbalance in prism2_bss_list_proc_start() - wrong count at exit
>
> The root cause is the missing annotations at prism2_bss_list_proc_start()
>
> Add the missing __acquires(&local->lock) annotation
> Add the missing __releases(&local->lock) annotation
>
> Signed-off-by: Jules Irenge <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

1c0e3c73e98d hostap: Add missing annotations for prism2_bss_list_proc_start() and prism2_bss_list_proc_stop

--
https://patchwork.kernel.org/patch/11483853/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2020-04-24 17:01:47

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 7/9] scsi: bnx2fc: Add missing annotation for bnx2fc_abts_cleanup()


Jules,

> Sparse reports a warning at bnx2fc_abts_cleanup()
>
> warning: context imbalance in bnx2fc_abts_cleanup() - unexpected unlock
>
> The root cause is the missing annotation at bnx2fc_abts_cleanup()
>
> Add the missing __must_hold(&tgt->tgt_lock) annotation

Applied to 5.8/scsi-queue, thanks!

--
Martin K. Petersen Oracle Linux Engineering