Messages in this thread | | | Date | Mon, 24 Aug 2009 15:40:30 -0700 (PDT) | From | David Rientjes <> | Subject | Re: [patch 3/4 -mm] flex_array: poison free elements |
| |
On Mon, 24 Aug 2009, Dave Hansen wrote:
> On Mon, 2009-08-24 at 13:41 -0700, David Rientjes wrote: > > LEX_ARRAY_FREE (or an additional poison value to > > distinguish between use-uninitialized vs. use-after-free) must be used in > > flex_array_clear() otherwise the cgroup patchset, the only proposed user > > of this library code, could never shrink this array when pid's are free > > like the kmalloc vs. vmalloc patchset could do. > > Are you saying that you expected it to never reallocate the array, but > have a permanent flex_array and that it just calls flex_array_clear() on > the elements that it doesn't want any more, and the array ends up > sparsely populated? I can see why we'd need a poison value in that > case. >
Yeah, similar to using flex_array_free_parts() with a statically allocated struct flex_array storing the array's metadata (which is also broken, by the way, since it does no sanity check on either parameter in FLEX_ARRAY_INIT()).
> Or, are we just talking about a situation where we need to truncate the > pidlist? >
The patchset Andrew referred to where he proposed using flex array instead would re-kmalloc (or re-vmalloc, I guess) a smaller amount of memory and then transfer elements into it to free the larger set of pages. If they were to convert to using flex array, this would be achieved with flex_array_shrink() so they lose no functionality with the switch.
I don't think they can use flex array for another reason, however, because they want to sort the pidlist as its returned to userspace. That would require dynamically allocating almost the same amount of memory as the flex array would, sorting it, and then storing it in the array. This doesn't seem to be the best example use of flex array since the mapping of array indices to pid isn't really relevant and there's a longer iteration through the array if it is sparsely populated, which is why I suggested an alternative implementation of flex list.
| |