[PATCH 0/9] Lock warning cleanups

[PATCH 0/9] Lock warning cleanups

[Jules Irenge]

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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

[PATCH 2/9] mt76: remove unnecessary annotations

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

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

[Kalle Valo]

Jules Irenge <jbi.octave@gmail.com> 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 <jbi.octave@gmail.com>

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

[PATCH 4/9] mac80211: Add missing annotation for brcms_rfkill_set_hw_state()

[Jules Irenge]

Sparse reports a warning at brcms_rfkill_set_hw_state()

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

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 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 8e8b685cfe09..c3dbeacea6ca 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
@@ -1717,6 +1717,7 @@ int brcms_check_firmwares(struct brcms_info *wl)
  * precondition: perimeter lock has been acquired
  */
 bool brcms_rfkill_set_hw_state(struct brcms_info *wl)
+	__must_hold(&wl->lock)
 {
 	bool blocked = brcms_c_check_radio_disabled(wl->wlc);
 
-- 
2.24.1

Re: [PATCH 4/9] brcmsmac: Add missing annotation for brcms_rfkill_set_hw_state()

[Kalle Valo]

Jules Irenge <jbi.octave@gmail.com> wrote:

> Sparse reports a warning at brcms_rfkill_set_hw_state()
> 
> warning: context imbalance in brcms_rfkill_set_hw_state()
> 	- unexpected unlock
> The root cause is the missing annotation at brcms_rfkill_set_hw_state()
> Add the missing __must_hold(&wl->lock) annotation
> 
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>

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

2fe5efb8a475 brcmsmac: Add missing annotation for brcms_rfkill_set_hw_state()

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

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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

Re: [PATCH 5/9] brcmsmac: Add missing annotation for brcms_down()

[Kalle Valo]

Jules Irenge <jbi.octave@gmail.com> wrote:

> 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 <jbi.octave@gmail.com>

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

40fb232c02d1 brcmsmac: Add missing annotation for brcms_down()

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

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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

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

[John Garry]

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 <jbi.octave@gmail.com>

that looks ok...

Reviewed-by: John Garry <john.garry@huawei.com>

> ---
>   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;
> 

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

[Martin K. Petersen]

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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

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

[Martin K. Petersen]

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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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

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

[Jules Irenge]

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 <jbi.octave@gmail.com>
---
 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