public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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).

  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