Re: [PATCH v4 12/18] xen/pvcalls: implement poll command

From: Boris Ostrovsky
Date: Wed Jun 21 2017 - 16:55:31 EST



>>> +
>>> + mappass->reqcopy = *req;
>>> + icsk = inet_csk(mappass->sock->sk);
>>> + queue = &icsk->icsk_accept_queue;
>>> + spin_lock(&queue->rskq_lock);
>>> + data = queue->rskq_accept_head != NULL;
>>> + spin_unlock(&queue->rskq_lock);
>> What is the purpose of the queue lock here?
> It is only there to protect accesses to rskq_accept_head. Functions that
> change rskq_accept_head take this lock, see for example
> net/ipv4/inet_connection_sock.c:inet_csk_reqsk_queue_add. I'll add an
> in-code comment.

I am not sure I follow. You are not changing rskq_accept_head, you are
simply reading it under the lock. It may be set by others to NULL as
soon as you drop the lock, at which point 'data' test below will be
obsolete.

In inet_csk_reqsk_queue_add() it is read and then, based on read result,
is written with a value so a lock is indeed need there.

-boris

>
>
>>> + if (data) {
>>> + mappass->reqcopy.cmd = 0;
>>> + ret = 0;
>>> + goto out;
>>> + }
>>> + spin_unlock_irqrestore(&mappass->copy_lock, flags);
>>> +
>>> + /* Tell the caller we don't need to send back a notification yet */
>>> + return -1;