This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Wed, Aug 19, 2015 at 02:53:06PM -0400, Mike Frysinger wrote: > i'm not sure this fixes the stated bug. say you have two arenas: > a -> b -> a > b is corrupt and a is avoided. > avoid_arena = a; > result = a; > begin = a; > > so the first loop runs because result == avoid_arena, so we move result=b, > and then the second loop runs because arena_is_corrupt(b), so we move > result=a, and then we hit the if statement which sets looped=true, and then > we break out. then we return NULL even though there is a non-corrupt region. > > looks like this applies whenever the first region is avoided and the others > are corrupt. Thanks for pointing that out, but it appears to me that this problem is only valid when none of the arenas are corrupt and no arenas are avoided. One would not really care about performance loss when any of the arenas are corrupt because it will just be a matter of time before the program dies. There is of course the case of: a -> a where one could fall back to mmap, but that's OK for two reasons: 1. It is unlikely because the code will always attempt to create another arena before going down the retry path, so you'd at least end up with a -> b -> a. In the unlikely case that creating the arena fails, we don't fall back to reusing anyway, and we just use mmap 2. Even if we ever end up with the a -> a case, it's OK to fall back because we don't really care about retrying in the same map that we failed, so I'd argue that the original code was wrong in returning the same arena. This also means that my original assertion about the "degenerate case being only one arena" is wrong; there is no such degenerate case. Also, the code around this is a bit murky because of some useless bits. I'll try to clean them up in follow-up patches. Is the explanation sound enough for me to commit this? I can add comments in the code to explain this in detail if you think it'll help. Siddhesh
Attachment:
pgpkq4l5tCXoo.pgp
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |