linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach()
@ 2025-04-23 15:08 Johannes Berg
  2025-04-25 10:20 ` Simon Horman
  2025-04-25 19:01 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Johannes Berg @ 2025-04-23 15:08 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-mediatek, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There are some sparse warnings in wifi, and it seems that
it's actually possible to annotate a function pointer with
__releases(), making the sparse warnings go away. In a way
that also serves as documentation that rcu_read_unlock()
must be called in the attach method, so add that annotation.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/soc/mediatek/mtk_wed.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h
index a476648858a6..d8949a4ed0dc 100644
--- a/include/linux/soc/mediatek/mtk_wed.h
+++ b/include/linux/soc/mediatek/mtk_wed.h
@@ -192,7 +192,7 @@ struct mtk_wed_device {
 };
 
 struct mtk_wed_ops {
-	int (*attach)(struct mtk_wed_device *dev);
+	int (*attach)(struct mtk_wed_device *dev) __releases(RCU);
 	int (*tx_ring_setup)(struct mtk_wed_device *dev, int ring,
 			     void __iomem *regs, bool reset);
 	int (*rx_ring_setup)(struct mtk_wed_device *dev, int ring,
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach()
  2025-04-23 15:08 [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach() Johannes Berg
@ 2025-04-25 10:20 ` Simon Horman
  2025-04-25 10:22   ` Johannes Berg
  2025-04-25 19:01 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2025-04-25 10:20 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev, linux-kernel, Matthias Brugger,
	AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
	Johannes Berg

On Wed, Apr 23, 2025 at 05:08:08PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There are some sparse warnings in wifi, and it seems that
> it's actually possible to annotate a function pointer with
> __releases(), making the sparse warnings go away. In a way
> that also serves as documentation that rcu_read_unlock()
> must be called in the attach method, so add that annotation.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Thinking out loud:

* Without this patch I see the following, but with this patch I do not.

  .../mt7915/mmio.c:636:5: warning: context imbalance in 'mt7915_mmio_wed_init' - wrong count at exit
  .../mt7996/mmio.c:302:5: warning: context imbalance in 'mt7996_mmio_wed_init' - wrong count at exit

* The only implementation of this callback I found is mtk_wed_attach
  which is already annotated as __releases(RCU);

* The only caller of this callback I could find is mtk_wed_device_attach()
  which takes rcu_read_unlock(). And the the callback needs to release it
  to avoid imbalance.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach()
  2025-04-25 10:20 ` Simon Horman
@ 2025-04-25 10:22   ` Johannes Berg
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2025-04-25 10:22 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, linux-kernel, Matthias Brugger,
	AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek

On Fri, 2025-04-25 at 11:20 +0100, Simon Horman wrote:
> On Wed, Apr 23, 2025 at 05:08:08PM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > There are some sparse warnings in wifi, and it seems that
> > it's actually possible to annotate a function pointer with
> > __releases(), making the sparse warnings go away. In a way
> > that also serves as documentation that rcu_read_unlock()
> > must be called in the attach method, so add that annotation.
> > 
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Thanks :) Looks like Jakub already applied it, FWIW.

> Thinking out loud:
> 
> * Without this patch I see the following, but with this patch I do not.
> 
>   .../mt7915/mmio.c:636:5: warning: context imbalance in 'mt7915_mmio_wed_init' - wrong count at exit
>   .../mt7996/mmio.c:302:5: warning: context imbalance in 'mt7996_mmio_wed_init' - wrong count at exit

Right, that's what I was trying to get rid of.

> * The only implementation of this callback I found is mtk_wed_attach
>   which is already annotated as __releases(RCU);

Indeed, but sparse doesn't check that for function pointer
compatibility.

> * The only caller of this callback I could find is mtk_wed_device_attach()
>   which takes rcu_read_unlock(). And the the callback needs to release it
>   to avoid imbalance.
> 

Right, pretty sure that's all intentional (though I don't understand
it), hence this change to document it a bit better and get rid of the
warnings.

johannes


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach()
  2025-04-23 15:08 [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach() Johannes Berg
  2025-04-25 10:20 ` Simon Horman
@ 2025-04-25 19:01 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-25 19:01 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev, linux-kernel, matthias.bgg, angelogioacchino.delregno,
	linux-arm-kernel, linux-mediatek, johannes.berg

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 23 Apr 2025 17:08:08 +0200 you wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There are some sparse warnings in wifi, and it seems that
> it's actually possible to annotate a function pointer with
> __releases(), making the sparse warnings go away. In a way
> that also serves as documentation that rcu_read_unlock()
> must be called in the attach method, so add that annotation.
> 
> [...]

Here is the summary with links:
  - [net-next] net: ethernet: mtk_wed: annotate RCU release in attach()
    https://git.kernel.org/netdev/net-next/c/d57ee99831e3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-25 19:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 15:08 [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach() Johannes Berg
2025-04-25 10:20 ` Simon Horman
2025-04-25 10:22   ` Johannes Berg
2025-04-25 19:01 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).