On Wed, 27 Jul 2016 12:10:13 +0800 zhong jiang <zhongjiang@xxxxxxxxxx> wrote:
> >> + for (i = 0; i < nr_segments; i++) {
> >> + if (PAGE_COUNT(image->segment[i].memsz) > totalram_pages / 2
> >> + || PAGE_COUNT(total_segments) > totalram_pages / 2)
> >> + return result;
> > And I don't think we need this? Unless we're worried about the sum of
> > all segments overflowing an unsigned long, which I guess is possible.
> > But if we care about that we should handle it in the next statement:
> >
> >> + total_segments += image->segment[i].memsz;
> > Should this be
> >
> > total_pages += PAGE_COUNT(image->segment[i].memsz);
> ok
> > ? I think "yes", if the segments are allocated separately and "no" if
> > they are all allocated in a big blob.
> There is a possible that most of segments size will exceed half of the real memory.
>
> if (PAGE_COUNT(image->segment[i].memsz) > totalram_pages / 2
> || total_pages > totalram_pages / 2)
> I guess that it is ok , we should bail out timely when it happens to the condition.
>
> is right ?
>
> your mean that above condition is no need. In the end, we check the overflow just one time.
> or I misunderstand.
It doesn't matter much. Actually I misread the code a bit. How about
for (i = 0; i < nr_segments; i++) {
unsigned long seg_pages = PAGE_COUNT(image->segment[i].memsz);
if (seg_pages > totalram_pages / 2))
return -EINVAL;
total_pages += seg_pages;
if (total_pages > totalram_pages / 2)
return -EINVAL;
}
--
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
