Re: [PATCH RFC nf-next 10/12] netfilter: ipset: use correct lockdep annotation in ipset_dereference

Jozsef Kadlecsik <[email protected]> wrote:
> > ip_set_comment_free() is problematic: called from different contexts,
> > some hold set->lock spinlock (safe), some do not hold a lock but have other
> > means of mutual exclusion (e.g., entire set torn down).
> 
> Wouldn't something like the following be sufficient?
> 
> - Add a bool "deleted" element to struct ip_set.
> - The set-specific destroy function would set it true before
>   doing anything else.
> - Then it'd be safe to use

> #define ipset_dereference_locked(p, set)	\
> 	rcu_dereference_protected(p, lockdep_is_held(&set->lock) || \
> 				     set->deleted))

Yes, that would work.  I'll add this to my todo list.

> > Other callers need investigation: ip_set_comment_free() alters
> > set->ext_size in a non-atomic way.  I don't see how this is safe except
> > for "entire set is destroyed" case: parallel usage would be a bug.
> 
> Maybe we should convert ext_size to atomic64_t?

I was wondering that too.  It would be simpler and avoid these
new helpers -> less code churn.