Re: netlink: Add barrier to netlink_connect for theoretical case

[Tejun Heo <tj@kernel.org>]

Hello, Herbert.

On Fri, Sep 25, 2015 at 11:39:57AM +0800, Herbert Xu wrote:
> +static inline bool netlink_bound(struct netlink_sock *nlk)
> +{
> +	bool bound = READ_ONCE(nlk->bound);
> +
> +	/* Ensure nlk is hashed and visible. */
> +	if (bound)
> +		smp_rmb();
> +
> +	return bound;
> +}

While I can't see anything wrong with the above, I'm not a fan of it
for whatever worth that may be.  I don't think it adds anything in
terms of readability or clarity of the code.  It does avoid smp_rmb()
when @bound is false but that's unlikely to be helfpul - where the
barrier is being avoided is a cold path.  This is largely a generic
characteristic because if where the barrier is being avoided is a hot
path, why wouldn't the code just grab a lock in that path instead of
using a gated barrier?  So, there's a reason why we don't see code
like the above commonly.  It doesn't buy us anything meaningful while
making the code more complicated and sometimes more fragile.

Thanks.

-- 
tejun