On 6/24/19 7:11 AM, Florian Weimer wrote:
* Carlos O'Donell:
> I am fairly confident that the code works and doesn't cause serious
> problems, but I'd like a double check from both of you.
The new algorithm will not find a chunk if it has been consolidated with
its preceding chunk (at a lower address), but not with its successor.
This is true.
We could improve it to try look for this case.
I see 3 cases:
1- Consolidation has not happened, and we have the header, chunk, and
remainder, and chunk is aligned, we find it and we return.
2- Consolidation has happened and we have one large chunk again, and
it's unaligned, but we'll use it, and split it into a header, chunk
and remainder; returning the aligned chunk.
3- Consolidation has happened and we have consolidated only with the
header, and we have a perfect opportunity to split just the header
off.
We handle case 1 and 2, but not 3.
Did I describe your position correctly?
The unsorted bins aren't searched, either.
This is a potentially costly operation.
I expected that you would search bins which are larger than the
request
size, too, in case the aligned pointer is further into the allocation.
This is equivalent to case 3 above, you can think of it as a case where
consolidation occurred, and we might want to split into a header + chunk.
The list updates lack the usual security checks, and there are some
implicit null pointer checks (style-only issue).
We can clean these up with some subsequent refinements.
So let me summarize:
a- We support only the additional case 1, which is sufficiently sized and
aligned chunks.
b- We don't support case 3, which is a consolidated header and aligned
chunk.
c- We don't support scanning for chunks which fall inbetween a correctly
sized and aligned chunk, and a chunk that is sized correctly for the
aligned chunk to produce a header + footer only (no remainder, though
internal fragmentation can happen here if a remainder is too small to
be split out for chunk minsize). This kind of chunk is never returned
via the fallback naive method because it's not large enough.
d- New code needs more security checks.
e- New code has style issues.
Do we need to address any of (a) through (e) before we start testing this
in Rawhide?
We can't release F31 with this patch so we will have to back it out before
too long. Is this a problem given the imminent release? Do we need to wait
for F31 to branch before doing this kind of testing?
--
Cheers,
Carlos.