netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: pmullaney@novell.com
Cc: GHaskins@novell.com, netdev@vger.kernel.org
Subject: Re: [PATCH] net/core/sock.c remove extra wakeup
Date: Fri, 30 May 2008 02:52:54 -0700 (PDT)	[thread overview]
Message-ID: <20080530.025254.99938262.davem@davemloft.net> (raw)
In-Reply-To: <483E80AB020000C70003891C@lucius.provo.novell.com>

From: "Patrick Mullaney" <pmullaney@novell.com>
Date: Thu, 29 May 2008 10:08:43 -0600

> We have noticed an extra wakeup of a task waiting on a udp receive
> while testing with CONFIG_PREEMPT_RT. The following patch
> eliminates the source of the extra wakeup by only waking a task
> if it is waiting on the writable condition(the SOCK_NOSPACE
> bit has been set). The patch is against 2.6.25.
> 
> Signed-off-by: Patrick Mullaney <pmullaney@novell.com>

This looks Ok on the surface, but I'm really worried about
races.

If you clear that bit, another thread which set that bit
and already checked the socket space, might sleep and never
wake up.

I think that is fundamentally why we don't try to optimize
this in that way.

And why is this so expensive?  Once the first wakeup occurs,
the subsequent attemps will merely see that the wait queue
is empty and do nothing.

The only cost is the read lock on the callback lock, and that
is about as expensive as this new atomic operation you are
adding to clear the SOCK_NOSPACE bit.

So the benefit is not clear, and neither is the correctness of
this change.

Sorry.

  reply	other threads:[~2008-05-30  9:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-29 16:08 [PATCH] net/core/sock.c remove extra wakeup Patrick Mullaney
2008-05-30  9:52 ` David Miller [this message]
     [not found] <483F99090200005A00037FFE@sinclair.provo.novell.com>
2008-05-30 10:05 ` Gregory Haskins
2008-06-11  6:46   ` Herbert Xu
2008-06-16  3:48     ` Gregory Haskins
     [not found] <483F99090200005A00037FFE@lucius.provo.novell.com>
     [not found] ` <483F990C0200005A00038001@lucius.provo.novell.com>
2008-05-30 17:00   ` Patrick Mullaney
2008-06-01 21:03     ` Andi Kleen

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=20080530.025254.99938262.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=GHaskins@novell.com \
    --cc=netdev@vger.kernel.org \
    --cc=pmullaney@novell.com \
    /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;
as well as URLs for NNTP newsgroup(s).