From: Rainer Weikusat <rweikusat@mssgmbh.com>
To: David Miller <davem@davemloft.net>
Cc: rweikusat@mssgmbh.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2.6.25.7] af_unix: fix 'poll for write'/ connected DGRAM sockets
Date: Wed, 18 Jun 2008 14:31:07 +0200 [thread overview]
Message-ID: <874p7qsz5g.fsf@fever.mssgmbh.com> (raw)
In-Reply-To: <20080617.215630.47207590.davem@davemloft.net> (David Miller's message of "Tue, 17 Jun 2008 21:56:30 -0700 (PDT)")
David Miller <davem@davemloft.net> writes:
> From: Rainer Weikusat <rweikusat@mssgmbh.com>
> Date: Tue, 17 Jun 2008 20:47:02 +0200
>> The unix_dgram_sendmsg routine implements
[...]
> I'm going to review the logic in the new poll routing a little
> bit more, then apply it to net-2.6 unless I find some problems.
Thank you for having a look at this. I have found a couple of problems
myself in the meantime:
- I misread the check in unix_dgram_sendmsg, which guards the
'receive queue' test: That actually tests if the peer of the
other socket is not the sending socket itself, ie it is true
if the sending socket is either unconnected or the destination
socket is the 1 in a n:1-setup (The well-known example of
this would be /dev/log. This happened to be my use
scenario, too).
- Assuming the socket wasn't writeable because it still owns
too many datagrams, it shouldn't be but onto the peer_wait
queue of the peer. If the destination socket consumes one of
the datagrams credited to the sending socket, the thread
will be woken up 'as usual'. Otherwise, there is no point in
waking it at all.
- Splitting the 'check for writeable' code in two parts, one
above and one below the other checks, was a stupid idea: The
only requirement is that the thread is added to the other
wait queue before it checks the state of the peer's
sk_receive_queue and I think having all this code together
makes it easier to understand.
- The naming is inconsistent with the other datagram socket
routines.
I will submit an updated patch which addresses this.
A couple of related-but-different oddities:
- What happens if a thread is blocked in poll and another
thread re-connects the socket to someone else? This should
presumably cause the other thread to be woken up if it is
presently queued on the peer wait queue.
- What happens if someone connects the other socket? Presently
(insofar I understand the code correctly),
nothing. unix_dgram_connect just sets the peer of a socket
which was unconnected before, despite its receive queue
may still contain packets sent to it before the connect,
which it (according to the comment above _disconnect),
shouldn't receive anymore (I have tested that this actually
happens with the help of perl).
next prev parent reply other threads:[~2008-06-18 12:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-17 18:47 [PATCH 2.6.25.7] af_unix: fix 'poll for write'/ connected DGRAM sockets Rainer Weikusat
2008-06-18 4:56 ` David Miller
2008-06-18 12:31 ` Rainer Weikusat [this message]
2008-06-18 21:48 ` David Miller
2008-06-19 14:14 ` [PATCH 2.6.25.7 v1-v2] af_unix: fix 'poll for write'/connected " Rainer Weikusat
[not found] ` <20080619.161313.90488863.davem@davemloft.net>
2008-06-20 13:35 ` Rainer Weikusat
2008-06-28 2:34 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874p7qsz5g.fsf@fever.mssgmbh.com \
--to=rweikusat@mssgmbh.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox