[PATCH 0/6] lock warning cleanups

[PATCH 0/6] 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 (6):
  namei: Add missing annotation for unlazy_walk()
  namei: Add missing annotation for unlazy_child()
  fs: Add missing annotation for __wait_on_freeing_inode()
  fs/fs-writeback.c: Add missing annotation for
    wbc_attach_and_unlock_inode()
  fs/fs-writeback.c: replace two annotations for
    __inode_wait_for_writeback()
  fs: add missing annotation for pin_kill()

 fs/fs-writeback.c | 4 ++--
 fs/fs_pin.c       | 1 +
 fs/inode.c        | 2 ++
 fs/namei.c        | 2 ++
 4 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.24.1

[PATCH 1/6] namei: Add missing annotation for unlazy_walk()

[Jules Irenge]

Sparse reports a warning at unlazy_walk()

warning: context imbalance in unlazy_walk() - unexpected unlock

The root cause is the missing annotation at unlazy_walk()

Add the missing __releases(RCU) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/namei.c b/fs/namei.c
index db6565c99825..d80e1ac8c211 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -678,6 +678,7 @@ static bool legitimize_root(struct nameidata *nd)
  * terminate_walk().
  */
 static int unlazy_walk(struct nameidata *nd)
+	__releases(RCU)
 {
 	struct dentry *parent = nd->path.dentry;
 
-- 
2.24.1

[PATCH 2/6] namei: Add missing annotation for unlazy_child()

[Jules Irenge]

Sparse reports a warning at unlazy_child()

warning: context imbalance in unlazy_child() - unexpected unlock

The root cause is the missing annotation at unlazy_walk()

Add the missing __releases(RCU) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/namei.c b/fs/namei.c
index d80e1ac8c211..9af3e8e438a1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -717,6 +717,7 @@ static int unlazy_walk(struct nameidata *nd)
  * terminate_walk().
  */
 static int unlazy_child(struct nameidata *nd, struct dentry *dentry, unsigned seq)
+	__releases(RCU)
 {
 	BUG_ON(!(nd->flags & LOOKUP_RCU));
 
-- 
2.24.1

[PATCH 3/6] fs: Add missing annotation for __wait_on_freeing_inode()

[Jules Irenge]

Sparse reports a warning at __wait_on_freeing_inode()

warning: context imbalance in __wait_on_freeing_inode() - unexpected unlock

The root cause is the missing annotations at __wait_on_freeing_inode()

Add the missing __releases(&inode->i_lock)
	and __must_hold(&inode_hash_lock) annotations

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/inode.c b/fs/inode.c
index 7d57068b6b7a..3b06c5c59883 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1955,6 +1955,8 @@ EXPORT_SYMBOL(inode_needs_sync);
  * will DTRT.
  */
 static void __wait_on_freeing_inode(struct inode *inode)
+	__releases(&inode->i_lock)
+	__must_hold(&inode_hash_lock)
 {
 	wait_queue_head_t *wq;
 	DEFINE_WAIT_BIT(wait, &inode->i_state, __I_NEW);
-- 
2.24.1

[PATCH 4/6] fs/fs-writeback.c: Add missing annotation for wbc_attach_and_unlock_inode()

[Jules Irenge]

Sparse reports a warning at wbc_attach_and_unlock_inode()

warning: context imbalance in wbc_attach_and_unlock_inode() - unexpected unlock

The root cause is the missing annotation at wbc_attach_and_unlock_inode()
Add the missing __releases(&inode->i_lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/fs-writeback.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 76ac9c7d32ec..008e572faf32 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -556,6 +556,7 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
  */
 void wbc_attach_and_unlock_inode(struct writeback_control *wbc,
 				 struct inode *inode)
+	__releases(&inode->i_lock)
 {
 	if (!inode_cgwb_enabled(inode)) {
 		spin_unlock(&inode->i_lock);
-- 
2.24.1

[PATCH 5/6] fs/fs-writeback.c: replace two annotations for __inode_wait_for_writeback()

[Jules Irenge]

To improve readability of the code,
__releases(inode->i_lock) and __acquires(inode->i_lock) annotations here
are replaced by one annotation __must_hold(inode->i_lock)

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/fs-writeback.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 008e572faf32..d0c0dc10526d 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1322,8 +1322,7 @@ static int write_inode(struct inode *inode, struct writeback_control *wbc)
  * Caller must make sure inode cannot go away when we drop i_lock.
  */
 static void __inode_wait_for_writeback(struct inode *inode)
-	__releases(inode->i_lock)
-	__acquires(inode->i_lock)
+	__must_hold(inode->i_lock)
 {
 	DEFINE_WAIT_BIT(wq, &inode->i_state, __I_SYNC);
 	wait_queue_head_t *wqh;
-- 
2.24.1

[PATCH 6/6] fs: add missing annotation for pin_kill()

[Jules Irenge]

Sparse reports a warning at pin_kill()

warning: context imbalance in pin_kill() - unexpected unlock

The root cause is the missing annotation at pin_kill()

Add the missing __releases(RCU)

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 fs/fs_pin.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fs_pin.c b/fs/fs_pin.c
index 47ef3c71ce90..ee57700740df 100644
--- a/fs/fs_pin.c
+++ b/fs/fs_pin.c
@@ -28,6 +28,7 @@ void pin_insert(struct fs_pin *pin, struct vfsmount *m)
 }
 
 void pin_kill(struct fs_pin *p)
+	__releases(RCU)
 {
 	wait_queue_entry_t wait;
 
-- 
2.24.1

[PATCH 0/6] Lock warnings cleanup

[Jules Irenge]

This patch serie propose a solution to functions that record context
imbalance by adding annotations

Jules Irenge (6):
  usb: gadget: Add missing annotation for xudc_handle_setup()
  udp: Add missing annotations for busylock_acquire() and
    busylock_release()
  udp: Add annotations for udp_rmem_release()
  net: atm: Add missing annotation for lec_seq_stop()
  USB: dummy-hcd: Add missing annotation for set_link_state()
  net: atm: Add annotation for lec_priv_walk()

 drivers/usb/gadget/udc/dummy_hcd.c  |  1 +
 drivers/usb/gadget/udc/udc-xilinx.c |  1 +
 net/atm/lec.c                       | 11 ++++++++++-
 net/ipv4/udp.c                      | 11 +++++++++++
 4 files changed, 23 insertions(+), 1 deletion(-)

-- 
2.25.3

[PATCH 1/6] usb: gadget: Add missing annotation for xudc_handle_setup()

[Jules Irenge]

Sparse reports a warning at xudc_handle_setup()

warning: context imbalance in xudc_handle_setup() - unexpected unlock

The root cause is the missing annotation at xudc_handle_setup()

Add the missing __must_hold(&udc->lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/usb/gadget/udc/udc-xilinx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index 29d8e5f8bb58..ae22882dba6a 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -1733,6 +1733,7 @@ static void xudc_set_clear_feature(struct xusb_udc *udc)
  * Process setup packet and delegate to gadget layer.
  */
 static void xudc_handle_setup(struct xusb_udc *udc)
+	__must_hold(&udc->lock)
 {
 	struct xusb_ep *ep0 = &udc->ep[0];
 	struct usb_ctrlrequest setup;
-- 
2.25.3

[PATCH 2/6] udp: Add missing annotations for busylock_acquire() and busylock_release()

[Jules Irenge]

Sparse reports warnings

warning: context imbalance in busylock_acquire() - wrong count at exit
warning: context imbalance in busylock_release() - unexpected unlock

The root cause is the missing annotations at
busylock_acquire() and busylock_release()

The __release(busy) annotation inside busylock_release()
tells Sparse and not GCC to shutdown the warning
in case the condition is not satisfied.

Add the missing __acquires(busy) annotation
Add the missing __releases(busy) annotation
Add the __release(busy) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 net/ipv4/udp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index db76b9609299..5ca12a945ac3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1430,6 +1430,7 @@ static int udp_busylocks_log __read_mostly;
 static spinlock_t *udp_busylocks __read_mostly;
 
 static spinlock_t *busylock_acquire(void *ptr)
+	__acquires(busy)
 {
 	spinlock_t *busy;
 
@@ -1439,9 +1440,13 @@ static spinlock_t *busylock_acquire(void *ptr)
 }
 
 static void busylock_release(spinlock_t *busy)
+	__releases(busy)
 {
 	if (busy)
 		spin_unlock(busy);
+	else
+		/* annotation for sparse */
+		__release(busy);
 }
 
 int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
-- 
2.25.3

[PATCH 3/6] udp: Add annotations for udp_rmem_release()

[Jules Irenge]

Sparse reports a warning

 warning: context imbalance in udp_rmem_release() - unexpected unlock

To fix this,
__acquire(&sk_queue->lock) and __release(&sk_queue->lock) annotations
are added in case the condition is not met.

This add basically tell Sparse and not GCC to shutdown the warning

Add __acquire(&sk_queue->lock) annotation
Add the __release(&sk_queue->lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 net/ipv4/udp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5ca12a945ac3..175bd14bfac8 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1382,6 +1382,9 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
 	sk_queue = &sk->sk_receive_queue;
 	if (!rx_queue_lock_held)
 		spin_lock(&sk_queue->lock);
+	else
+		/* annotation for sparse */
+		__acquire(&sk_queue->lock);
 
 
 	sk->sk_forward_alloc += size;
@@ -1398,6 +1401,9 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
 
 	if (!rx_queue_lock_held)
 		spin_unlock(&sk_queue->lock);
+	else
+		/* annotation for sparse */
+		__release(&sk_queue->lock);
 }
 
 /* Note: called with reader_queue.lock held.
-- 
2.25.3

[PATCH 4/6] net: atm: Add missing annotation for lec_seq_stop()

[Jules Irenge]

Sparse reports a warning at lec_seq_stop()
warning: context imbalance in lec_seq_stop() - unexpected unlock

The root cause is the missing annotation at lec_seq_stop()
The __release() annotation inside lec_seq_stop()
tells only Sparse to shutdown the warning

Add the missing __releases(&state->locked->lec_arp_lock)
Add   __release(&state->locked->lec_arp_lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 net/atm/lec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 25fa3a7b72bd..22415bc11878 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -940,6 +940,7 @@ static void *lec_seq_start(struct seq_file *seq, loff_t *pos)
 }
 
 static void lec_seq_stop(struct seq_file *seq, void *v)
+	__releases(&state->locked->lec_arp_lock)
 {
 	struct lec_state *state = seq->private;
 
@@ -947,7 +948,9 @@ static void lec_seq_stop(struct seq_file *seq, void *v)
 		spin_unlock_irqrestore(&state->locked->lec_arp_lock,
 				       state->flags);
 		dev_put(state->dev);
-	}
+	} else
+		/* annotation for sparse */
+		__release(&state->locked->lec_arp_lock);
 }
 
 static void *lec_seq_next(struct seq_file *seq, void *v, loff_t *pos)
-- 
2.25.3

[PATCH 5/6] USB: dummy-hcd: Add missing annotation for set_link_state()

[Jules Irenge]

Sparse reports a warning at set_link_state()

warning: context imbalance in set_link_state() - unexpected unlock

The root cause is the missing annotation at set_link_state()
Add the missing __must_hold(&dum->lock)

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 4c9d1e49d5ed..c07ddf11e137 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -427,6 +427,7 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd)
 
 /* caller must hold lock */
 static void set_link_state(struct dummy_hcd *dum_hcd)
+	__must_hold(&dum->lock)
 {
 	struct dummy *dum = dum_hcd->dum;
 	unsigned int power_bit;
-- 
2.25.3

[PATCH 6/6] net: atm: Add annotation for lec_priv_walk()

[Jules Irenge]

Sparse reports a warning at lec_priv_walk()
warning: context imbalance in lec_priv_walk() - unexpected unlock

The root cause is the missing annotation at lec_priv_walk()
To fix this, __acquire() and __release() annotations
are added in case conditions are not met.
This only instruct  Sparse to shutdown the warning

Add the  __acquire(&priv->lec_arp_lock)
Add __release(&priv->lec_arp_lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 net/atm/lec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 22415bc11878..6070acaa3d5c 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -886,12 +886,18 @@ static void *lec_priv_walk(struct lec_state *state, loff_t *l,
 	if (!state->locked) {
 		state->locked = priv;
 		spin_lock_irqsave(&priv->lec_arp_lock, state->flags);
+	} else {
+		/* annotation for sparse */
+		__acquire(&priv->lec_arp_lock);
 	}
 	if (!lec_arp_walk(state, l, priv) && !lec_misc_walk(state, l, priv)) {
 		spin_unlock_irqrestore(&priv->lec_arp_lock, state->flags);
 		state->locked = NULL;
 		/* Partial state reset for the next time we get called */
 		state->arp_table = state->misc_table = 0;
+	} else {
+		/* annotation for sparse */
+		__release(&priv->lec_arp_lock);
 	}
 	return state->locked;
 }
-- 
2.25.3

Re: [PATCH 6/6] net: atm: Add annotation for lec_priv_walk()

[Andy Shevchenko]

On Wed, Apr 29, 2020 at 11:05:28AM +0100, Jules Irenge wrote:
> Sparse reports a warning at lec_priv_walk()
> warning: context imbalance in lec_priv_walk() - unexpected unlock
> 
> The root cause is the missing annotation at lec_priv_walk()
> To fix this, __acquire() and __release() annotations
> are added in case conditions are not met.
> This only instruct  Sparse to shutdown the warning
> 
> Add the  __acquire(&priv->lec_arp_lock)
> Add __release(&priv->lec_arp_lock) annotation

At least those two I got should be in one patch. Simple fix all same sparse
issues at once.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] net: atm: Add annotation for lec_priv_walk() and lec_seq_stop()

[Jules Irenge]

Sparse reports a warning at lec_priv_walk() and lec_seq_stop()

warning: context imbalance in lec_priv_walk() - unexpected unlock
warning: context imbalance in lec_seq_stop() - unexpected unlock

The root cause is the missing annotation at lec_priv_walk()
	and lec_seq_stop()
To fix this, __acquire() and __release() annotations
are added in case conditions are not met.
This only instruct Sparse to shutdown the warning

Add the  __acquire(&priv->lec_arp_lock)
Add __release(&priv->lec_arp_lock) annotation
Add __releases(&state->locked->lec_arp_lock) annotation
Add  __release(&state->locked->lec_arp_lock) annotation

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
Changes since v2 
- merge patch No 6 and No 4 into one

 net/atm/lec.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 25fa3a7b72bd..7947abb17af2 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -886,12 +886,18 @@ static void *lec_priv_walk(struct lec_state *state, loff_t *l,
 	if (!state->locked) {
 		state->locked = priv;
 		spin_lock_irqsave(&priv->lec_arp_lock, state->flags);
+	} else {
+		/* annotation for sparse */
+		__acquire(&priv->lec_arp_lock);
 	}
 	if (!lec_arp_walk(state, l, priv) && !lec_misc_walk(state, l, priv)) {
 		spin_unlock_irqrestore(&priv->lec_arp_lock, state->flags);
 		state->locked = NULL;
 		/* Partial state reset for the next time we get called */
 		state->arp_table = state->misc_table = 0;
+	} else {
+		/* annotation for sparse */
+		__release(&priv->lec_arp_lock);
 	}
 	return state->locked;
 }
@@ -940,6 +946,7 @@ static void *lec_seq_start(struct seq_file *seq, loff_t *pos)
 }
 
 static void lec_seq_stop(struct seq_file *seq, void *v)
+	__releases(&state->locked->lec_arp_lock)
 {
 	struct lec_state *state = seq->private;
 
@@ -947,6 +954,9 @@ static void lec_seq_stop(struct seq_file *seq, void *v)
 		spin_unlock_irqrestore(&state->locked->lec_arp_lock,
 				       state->flags);
 		dev_put(state->dev);
+	} else {
+		/* annotation for sparse */
+		__release(&state->locked->lec_arp_lock);
 	}
 }
 
-- 
2.25.3