This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 1/3] Fix __check_pf()/make_request() stack overflow segfault (convert to malloc)
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: "Banerjee, Debabrata" <dbanerje at akamai dot com>
- Cc: "Pengcheng dot Chen at gmail dot com" <Pengcheng dot Chen at gmail dot com>, "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Carlos O'Donell <carlos at redhat dot com>
- Date: Sat, 18 Jan 2014 02:06:18 +0100
- Subject: Re: [PATCH v2 1/3] Fix __check_pf()/make_request() stack overflow segfault (convert to malloc)
- Authentication-results: sourceware.org; auth=none
- References: <524E4504 dot 6050603 at redhat dot com> <1383268213-14349-1-git-send-email-dbanerje at akamai dot com> <20140116225341 dot GA23189 at domone dot podge> <CEFDCD71 dot 2C670%dbanerje at akamai dot com> <20140117002246 dot GA14011 at domone dot podge> <CEFF0C26 dot 2C764%dbanerje at akamai dot com>
On Fri, Jan 17, 2014 at 04:39:18PM -0500, Banerjee, Debabrata wrote:
> On 1/16/14, 7:22 PM, "OndÅej BÃlka" <neleai@seznam.cz> wrote:
>
> >On Thu, Jan 16, 2014 at 05:58:31PM -0500, Banerjee, Debabrata wrote:
> >>
> >>
> >> Sorry I haven't had time to look into refactoring this patch more.
> >>However
> >> Pengcheng has volunteered to clean it up in whatever way is acceptable.
> >>
> >> The caveat with using malloc is that I would expect it to be
> >>significantly
> >> slower than alloca, but someone can benchmark that. I believe this is a
> >> hot path.
> >>
> >That is tricky question, bottleneck could also be in network
> >latency, probably here
> >
> > if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
> > (struct sockaddr *) &nladdr,
> > sizeof (nladdr))) < 0)
>
> That's a netlink connection to the kernel, no point in worrying about
> latency here.
>
> >A better way would be to malloc result at start and write into in6ai
> >array directly, calling realloc to double size as necessary. At end we
> >could optionally trim memory at end.
>
> Less copies are good but multiple mallocs are bad.
Doubling allocation sizes is only rarely a performance problem. Main trick is to
start with sufficiently large buffer (like 4096 bytes). Then the cost of
realloc is mostly in copying data. When you sum data copied by realloc
its at most twice than data used. And when implementation needs to
initialize each byte its quite hard to be faster than copying. Typically
this is less than quarter of function runtime.
What could most likely cause performance problem is trimming realloc
when requests are small.
> What I wonder (and have
> spent no time looking at) is if getting all this data is necessary.
> Clearly the end result only uses a small fraction of this data. I wonder
> if any of this is necessary. My only goal with my patch was to fix the
> implementation of what was there, assuming it is necessary.
>
That is possible but would be harder to review.