From: Eric Dumazet <dada1@cosmosbay•com>
To: Evgeniy Polyakov <zbr@ioremap•net>
Cc: Stephen Hemminger <shemminger@vyatta•com>,
Herbert Xu <herbert@gondor•apana.org.au>,
berrange@redhat•com, et-mgmt-tools@redhat•com,
davem@davemloft•net, netdev@vger•kernel.org
Subject: Re: virt-manager broken by bind(0) in net-next.
Date: Sat, 31 Jan 2009 10:49:00 +0100 [thread overview]
Message-ID: <49841E8C.60401@cosmosbay.com> (raw)
In-Reply-To: <20090131093123.GA28151@ioremap.net>
Evgeniy Polyakov a écrit :
> On Sat, Jan 31, 2009 at 10:17:44AM +0100, Eric Dumazet (dada1@cosmosbay•com) wrote:
>>> getaddrinfo() returns list of addresses and IPv6 was the first one iirc.
>>> Previously it bailed out, but with my change it will try again without
>>> reason for doing this. With the patch I sent based on Eric's observation
>>> things should be fine.
>> Problem is your patch is wrong Evgeniy, please think about it litle bit more
>> and resubmit it.
>
> No, patch should be ok. And its part which moves bsockets around was
> added because of your complaints, that it is written into read-mostly
> cache line. It is not a fix and has nothing with the problem at all.
>
>> Take the time to run this $0.02 program, before and after your upcoming fix :
>
> It is not a fix, but enhancement, which really has nothing with the bug
> in question :)
> Fix is to return an error if socket binding does not use the heuristic.
>
>> offset of bsockets being 0x18 or 0x20 is same result : bad because in
>> same cache line than ehash, ehash_locks, ehash_size, ehash_locks_mask,
>> bhash, bhash_size, unless your cpu is a Pentium.
>
> Attached patch makes difference, I'm curious if it ever make any
> difference in the benchmarks.
>
>> Also, I suggest you change bsockets to something more appropriate, eg a
>> percpu counter.
>
> I thought on that first, but found that looping over every cpu and
> summing the total number of allocated/freed sockets will have noticebly
> bigger overhead than having loosely maintaned number of sockets.
>
> For the reference. This patch has nothing with the bug we discuss here,
> the proper patch (without need to move bsockets around) was sent
> earlier, which forces port selection codepath to return error when new
> selection heuristic is not used.
>
> --- ./include/net/inet_hashtables.h.orig 2009-01-31 12:27:41.000000000 +0300
> +++ ./include/net/inet_hashtables.h 2009-01-31 12:28:15.000000000 +0300
> @@ -134,7 +134,6 @@
> struct inet_bind_hashbucket *bhash;
>
> unsigned int bhash_size;
> - int bsockets;
>
> struct kmem_cache *bind_bucket_cachep;
>
> @@ -150,6 +149,8 @@
> */
> struct inet_listen_hashbucket listening_hash[INET_LHTABLE_SIZE]
> ____cacheline_aligned_in_smp;
> +
> + int bsockets ____cacheline_aligned_in_smp;
>
> };
>
>
It appears you are always right, I have nothing to say then.
Stupid I am.
I vote for plain revert of your initial patch, since you are anaware
of performance problems it introduces. Then, probably nobody cares
of my complaints, so dont worry.
next prev parent reply other threads:[~2009-01-31 9:49 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20090128212114.38be3e8c@extreme>
[not found] ` <20090129103544.GC22110@redhat.com>
2009-01-30 5:35 ` virt-manager broken by bind(0) in net-next Stephen Hemminger
2009-01-30 8:16 ` Evgeniy Polyakov
[not found] ` <20090130081600.GA2717-i6C2adt8DTjR7s880joybQ@public.gmane.org>
2009-01-30 10:27 ` Daniel P. Berrange
2009-01-30 11:21 ` Evgeniy Polyakov
2009-01-30 12:53 ` Herbert Xu
[not found] ` <20090130125337.GA7155-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
2009-01-30 17:57 ` Stephen Hemminger
2009-01-30 18:41 ` Eric Dumazet
2009-01-30 21:50 ` Evgeniy Polyakov
2009-01-30 22:30 ` Eric Dumazet
2009-01-30 22:51 ` Evgeniy Polyakov
[not found] ` <20090130225113.GA13977-i6C2adt8DTjR7s880joybQ@public.gmane.org>
2009-01-31 0:36 ` Stephen Hemminger
2009-01-31 8:35 ` Evgeniy Polyakov
2009-01-31 2:52 ` Stephen Hemminger
2009-01-31 8:37 ` Evgeniy Polyakov
2009-01-31 9:17 ` Eric Dumazet
2009-01-31 9:31 ` Evgeniy Polyakov
2009-01-31 9:49 ` Eric Dumazet [this message]
2009-01-31 9:56 ` Evgeniy Polyakov
2009-01-31 10:17 ` Eric Dumazet
2009-02-01 12:42 ` Evgeniy Polyakov
2009-02-01 16:12 ` Eric Dumazet
2009-02-01 17:40 ` Evgeniy Polyakov
2009-02-01 20:31 ` David Miller
[not found] ` <20090130215008.GB12210-i6C2adt8DTjR7s880joybQ@public.gmane.org>
2009-02-01 5:58 ` Stephen Hemminger
2009-02-01 9:07 ` David Miller
2009-02-01 12:44 ` Evgeniy Polyakov
[not found] ` <498349F7.4050300-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2009-02-01 5:29 ` Stephen Hemminger
2009-01-30 6:50 ` Stephen Hemminger
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=49841E8C.60401@cosmosbay.com \
--to=dada1@cosmosbay$(echo .)com \
--cc=berrange@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=et-mgmt-tools@redhat$(echo .)com \
--cc=herbert@gondor$(echo .)apana.org.au \
--cc=netdev@vger$(echo .)kernel.org \
--cc=shemminger@vyatta$(echo .)com \
--cc=zbr@ioremap$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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