Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path

[Stefano Brivio <sbrivio@redhat.com>]

Hi Vlad,

On Mon, 12 Nov 2018 09:55:45 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:

> @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work)
>  	rtnl_unlock();
>  }
>  
> +/* Helper function to lock rtnl mutex when specified condition is true and mutex
> + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock.
> + * Note that this function does nothing if rtnl is already held. This is
> + * intended to be used by cls API rules update API when multiple conditions
> + * could require rtnl lock and its state needs to be tracked to prevent trying
> + * to obtain lock multiple times.
> + */
> +
> +static void tcf_require_rtnl(bool cond, bool *rtnl_held)
> +{
> +	if (!*rtnl_held && cond) {
> +		*rtnl_held = true;
> +		rtnl_lock();
> +	}
> +}

I guess calls to this function are supposed to be serialised. If that's
the case (which is my tentative understanding so far), I would indicate
that in the comment.

If that's not the case, you would be introducing a race I guess.

Same applies to tcf_block_release() from 17/17.

-- 
Stefano