Re: [Patch net-next v6 1/7] r8169: add support for multi irqs

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 29 May 2026 05:43:52 +0000 Javen wrote:
> >devm_add_action_or_reset() can fail (as the AI bots point out) but this whole
> >devm_ dance is entirely unnecessary networking stack will automatically
> >delete NAPI instances when device is unregistered.  
> 
> Thanks for your review.
> 
> In patch v3, link: https://lore.kernel.org/netdev/20260513115543.1730-2-javen_xu@realsil.com.cn/
> I tried to alloc struct rtl8169_napi dynamically for saving memory according to Heiner's suggestion. I agree with his suggestion because only 8127 rss are enabled.
> And in this ai review, link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260520031603.700-1-javen_xu%40realsil.com.cn
> AI suggested that the lifetime of this devm_kcalloc'd napi array may be compatible with the netdev's napi list. So I add devm_add_action_or_reset in patch v6.
> I checked the code and agree that the stack auto-deletes NAPI instances in free_netdev() -> netdev_napi_exit(). However, because devres releases resources in LIFO order:
> 1. kfree for the NAPI array (allocated via devm_kcalloc) will be called first.
> 2. free_netdev() (registered via devm_alloc_etherdev) will be called second
> When free_netdev() calls netdev_napi_exit() to iterate over dev->napi_list, the NAPI memory has already been freed by devm, which will cause a Use-After-Free. That's why I added the devm action to explicitly remove it before the memory is freed.
> 
> So I wanna know what should I do? Whether keep the action in this
> patch(dynamically allocate napi array) or patch v2(fix the array
> size), or any other suggestion will be apperaciated.

Personal preference I guess. IMO it's pretty clear here that the devm_
help relatively little and they introduce a lot of complexity. You can
stop using them, or just handle the error on devm_add_action_or_reset().