Hi Dave,
On Thu, Aug 15, 2013 at 11:03:50AM -0700, Dave Hansen wrote:
>On 08/14/2013 05:31 PM, Wanpeng Li wrote:
>> After commit 9bdac91424075("sparsemem: Put mem map for one node together."),
>> vmemmap for one node will be allocated together, its logic is similiar as
>> memory allocation for pageblock flags. This patch introduce alloc_usemap_and_memmap
>> to extract the same logic of memory alloction for pageblock flags and vmemmap.
>
>Shame on whoever copy-n-pasted that in the first place.
>
>> -
>> - for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>> - struct mem_section *ms;
>> -
>> - if (!present_section_nr(pnum))
>> - continue;
>> - ms = __nr_to_section(pnum);
>> - nodeid_begin = sparse_early_nid(ms);
>> - pnum_begin = pnum;
>> - break;
>> - }
>> - usemap_count = 1;
>> - for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
>> - struct mem_section *ms;
>> - int nodeid;
>> -
>> - if (!present_section_nr(pnum))
>> - continue;
>> - ms = __nr_to_section(pnum);
>> - nodeid = sparse_early_nid(ms);
>> - if (nodeid == nodeid_begin) {
>> - usemap_count++;
>> - continue;
>> - }
>> - /* ok, we need to take cake of from pnum_begin to pnum - 1*/
>> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, pnum,
>> - usemap_count, nodeid_begin);
>> - /* new start, update count etc*/
>> - nodeid_begin = nodeid;
>> - pnum_begin = pnum;
>> - usemap_count = 1;
>> - }
>> - /* ok, last chunk */
>> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
>> - usemap_count, nodeid_begin);
>> + alloc_usemap_and_memmap(usemap_map, true);
>...
>> + alloc_usemap_and_memmap((unsigned long **)map_map, false);
>> #endif
>
>Why does alloc_usemap_and_memmap() take an 'unsigned long **'?
>'unsigned long' is for the usemap and 'struct page' is for the memmap.
>It's misleading to have it take an 'unsigned long **' and then just cast
>it over to a 'struct page **' internally.
>
Indeed.
>Also, what's the point of having a function that returns something in a
>double-pointer, but that doesn't use its return value?
>
So there is still need to cast return value since one is 'unsigned long
**' and the other is 'struct page **', do you have any idea to tackle
casting in this patch?
>alloc_usemap_and_memmap() also needs a comment about what it's doing
>with that pointer and its other argument.
>
I will write comment in next version, thanks for your point out. ;-)
Regards,
Wanpeng Li
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>