On Tue, Aug 30, 2016 at 08:05:03AM -0400, Jamal Hadi Salim wrote:
> On 16-08-30 07:03 AM, Amir Vadai wrote:
> > On Sun, Aug 28, 2016 at 10:04:21PM -0700, Cong Wang wrote:
> > > On Fri, Aug 26, 2016 at 12:16 PM, Eric Dumazet <[email protected]>
> > > wrote:
> > > > On Fri, 2016-08-26 at 11:26 -0700, Cong Wang wrote:
>
>
> > Regarding the specific action in this patchset, correct me if I'm wrong,
> > but I think that the lock could be removed safely.
> >
>
> From what Eric suggested (refer to my posting on skbmod),
> this becomes:
>
> +struct tcf_tunnel_key_p {
> + int tcft_action;
> + struct metadata_dst *tcft_enc_metadata;
> +};
>
> /* rcu protected */
> +struct tcf_tunnel_key {
> + struct tc_action common;
> + struct tcf_tunnel_key_p *p;
> +};
>
> At init() - always alloc struct tcf_tunnel_key_p, new
>
> old = rtnl_dereference(mykey->p);
> if (ovr)
> spin_lock_bh(&mykey->tcf_lock);
Thanks for the detailed example :)
what are we protecting with this spin lock here? isn't concurrent init()
calls are protected by the rtnl lock?
> ... update all params here ..
> rcu_assign_pointer(mykey->p, new);
> if (ovr) {
> spin_unlock_bh(&mykey->tcf_lock);
> synchronize_rcu();
> }
>
> kfree(old);
>
> at act():
>
> rcu_read_lock();
> struct tcf_tunnel_key_p *p = rcu_dereference(mykey->p);
> ... use p here ...
> rcu_read_unlock();
>
> Cong was looking to do something more generic for all actions.
>
> cheers,
> jamal
