Re: [PATCH v2] batman-adv: add dynamic, bridged-in TT VID detection support

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Thursday, 14 November 2024 14:58:43 CET Sven Eckelmann wrote:
> > Or maybe I missed something ?
> >
> > ---
> > net/batman-adv/soft-interface.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
> > index b61f35918b5d..d7de54734725 100644
> > --- a/net/batman-adv/soft-interface.c
> > +++ b/net/batman-adv/soft-interface.c
> > @@ -599,7 +599,6 @@ batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
> >
> > atomic_set(&vlan->ap_isolation, 0);
> >
> > -       kref_get(&vlan->refcount);
> > hlist_add_head_rcu(&vlan->list, &bat_priv->softif_vlan_list);
> > spin_unlock_bh(&bat_priv->softif_vlan_list_lock);
> >
> >
> 
> This ref is for the VLAN list entry (just one line below the kref_get).
> This patch is therefore definitely wrong.

Ok, had a look at the surrounding code from this patch and it looks too me 
like the reason for what a ref++ stands for was changed and so this 
initialization also needs to be changed. So, I have to correct my original 
statement about your patch, Nicholas. It is definitely an improvement to the 
refcnt (with this patch, not before). But at the same time, it should be made 
clear by Linus what the reference counter is used for. Especially because it 
was completely different before the patch and different then most other things 
in batman-adv.

Starting with not simply dropping the kref_get but with a small comment 
explaining that there is "no kref_get for list because only TT entries per 
VLAN and temporary references on stack are tracked. list entries will be 
removed automatically when refcount reaches zero".

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]