Re: [PATCH ipsec-next 01/10] xfrm: state: fix sparse warnings on xfrm_state_hold_rcu

[Steffen Klassert <steffen.klassert@secunet.com>]

On Tue, Mar 10, 2026 at 10:41:46PM +0100, Sabrina Dubroca wrote:
> 
> > Yes, this is possible scenario and this is what is worth to document.
> 
> 
> We could add something like:
> 
> /* Take a reference to @x, when we know the state has a refcount >= 1.
>  * In this case, we can avoid refcount_inc_not_zero and the error
>  * handling it requires.
>  * In contexts where concurrent state deletion is possible and we
>  * don't already hold a reference to that state, xfrm_state_hold_rcu
>  * must be used.
>  */
> 
> Though it may not make much sense to refer to xfrm_state_hold_rcu
> (implemented in net/xfrm/xfrm_state.c) from include/net/xfrm.h.
> 
> And if we consider the hashtables to be private to
> net/xfrm/xfrm_state.c, nothing outside of it should ever see a state
> with refcount=0, since they will only ever see states that already
> have one reference held by whatever gave them the pointer.
> 
> So maybe it's more xfrm_state_hold_rcu that needs a mention of
> "concurrent state deletion could bring the refcount to 0 while we're
> doing the lookup"? I don't know, for me it's pretty obvious with the
> _rcu suffix that RCU -> unlocked -> could be deleted in parallel.
> 
> 
> We also have __xfrm_state_put, the commit message that introduced it
> states:
> 
>     We often just do an atomic_dec(&x->refcnt) on an xfrm_state object
>     because we know there is more than 1 reference remaining and thus
>     we can elide the heavier xfrm_state_put() call.
> 
> so we could add:
> 
> /* Drop a reference to @x, when we know there is more than 1 reference remaining.
>  * In this case, we can avoid refcount_dec_and_test and just decrement refcnt.
>  */
> 
> but maybe someone has a better suggestion.

I plan to take this series as is. If you feel we need a comment,
just add it with a followup patch.