[MPTCP] Re: [PATCH net-next 6/8] mptcp: avoid work queue scheduling if possible

[Florian Westphal <fw at strlen.de>]

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

Paolo Abeni <pabeni(a)redhat.com> wrote:
> > +/* In most cases we will be able to lock the mptcp socket.  If its already
> > + * owned, we need to defer to the work queue to avoid ABBA deadlock.
> > + */
> > +static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
> > +{
> > +	struct sock *sk = (struct sock *)msk;
> > +	unsigned int moved = 0;
> > +
> > +	if (sock_owned_by_user_nocheck(sk))
> > +		return false;
> > +
> > +	if (unlikely(!spin_trylock_bh(&sk->sk_lock.slock)))
> > +		return false;
> > +
> > +	/* must re-check after taking the lock */
> > +	if (!sock_owned_by_user_nocheck(sk))
> 
> Don't we additionally need a READ_ONCE() here and above ?
> Otherwise the compiler and/or CPU could merge the read?

Oh, right, at least first check needs READ_ONCE annotation, thanks.

> AFAICS sock_owned_by_user_nocheck() does not include the additional
> annotation.
> 
> I think we could simply drop the first check (and always do the
> trylock)

The trylock would succeed however when socket is owned, leading to
a uneeded lock operation.