Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190412225100.GC23599@brightrain.aerifal.cx>
Date: Fri, 12 Apr 2019 18:51:00 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Cc: "zhangwentao (M)" <zhangwentao234@...wei.com>,
	"Jianing (OS-LAB)" <ning.jia@...wei.com>,
	"Huangqiang (H)" <h.huangqiang@...wei.com>,
	leijitang <leijitang@...wei.com>,
	wanghaozhan <wanghaozhan2@...wei.com>
Subject: Re: Re: 答复: musl:
 about malloc 'expand heap' issue

On Tue, Oct 30, 2018 at 11:44:36PM -0400, Rich Felker wrote:
> On Wed, Oct 31, 2018 at 01:19:11AM +0000, zhangwentao (M) wrote:
> > Hi
> > 
> >  Now we are using a global lock to solve the issue. And as you said the performance maybe cost too much.
> > And if you have solution about this and the fix is not that expensive, that's great.
> > If you finish it, would you send the patch to me ?
> 
> If I get a solution that's acceptable for upstream, it will be
> committed in git master and I'll follow up on this thread to mention
> it.
> 
> Unfortunately, looking again at where the spurious empty-bin situation
> happens (as a result of alloc_rev/_fwd during free), I don't see an
> easy fix that's not costly. The best we can do might be something like
> this:
> 
> In malloc, only use the per-bin lock to obtain a chunk if the
> exact-sized bin is non-empty. If it's empty, take a global lock
> (free_lock, but its semantics might need to be adjusted somewhat) to
> ensure a consistent view of what larger free chunks are available for
> splitting (which might end up revealing that a chunk of the exact
> desired bin was actually available). Ideally this will not have much
> impact on malloc performance under conditions where lots of free
> chunks are available (heavy load).

Here's a (WIP, with some debug mess left around) patch along the lines
I described above, to mitigate the badness of the current malloc. My
old malloc stress test, which is probably not representative of
real-world usage patterns, shows it being somewhat costly, but that's
expected, and is mainly an indication that the overall design is not
good. I'm attaching it here for initial review/comments, on the
existing thread since I know the ppl cc'd are interested. I'll follow
up in another thread later too.

In the process of doing this patch, I realized it probably is possible
to do the original dlmalloc-variant idea I had in mind before musl was
even released: putting locks in each chunk header/footer, and rather
than having a global split/merge lock, locking the header/footer
whenever we want to split or merge (change the sizes of) a chunk. I'm
still not 100% sure -- there's some locking order issue between the
ideal order for freeing/merging vs allocating/splitting. But it may be
a way to get some more life out of the old malloc, if it turns out the
attached patch is deemed too costly to apply as-is. It would also be a
good bit of work redesigning header structures, etc., at which point
it might make more sense to work on the new design instead.

Rich

View attachment "malloc_mitigations.diff" of type "text/plain" (9741 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.