On 2019-01-10 00:26, Andrew Morton wrote:
On Wed, 09 Jan 2019 16:36:36 +0530 Arun KS <aru...@codeaurora.org> wrote:

On 2019-01-09 16:27, Michal Hocko wrote:
> On Wed 09-01-19 16:12:48, Arun KS wrote:
> [...]
>> It will be called once per online of a section and the arg value is
>> always
>> set to 0 while entering online_pages_range.
>
> You rare right that this will be the case in the most simple scenario.
> But the point is that the callback can be called several times from
> walk_system_ram_range and then your current code wouldn't work
> properly.

Thanks. Will use +=

The v8 patch
https://lore.kernel.org/lkml/1547032395-24582-1-git-send-email-aru...@codeaurora.org/T/#u

(which you apparently sent 7 minutes after typing the above) still has

static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
                        void *arg)
 {
-       unsigned long i;
        unsigned long onlined_pages = *(unsigned long *)arg;
-       struct page *page;

        if (PageReserved(pfn_to_page(start_pfn)))
-               for (i = 0; i < nr_pages; i++) {
-                       page = pfn_to_page(start_pfn + i);
-                       (*online_page_callback)(page);
-                       onlined_pages++;
-               }
+               onlined_pages = online_pages_blocks(start_pfn, nr_pages);


Even then the code makes no sense.

static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
                        void *arg)
{
        unsigned long onlined_pages = *(unsigned long *)arg;

        if (PageReserved(pfn_to_page(start_pfn)))
                onlined_pages += online_pages_blocks(start_pfn, nr_pages);

        online_mem_sections(start_pfn, start_pfn + nr_pages);

        *(unsigned long *)arg += onlined_pages;
        return 0;
}

Either the final assignment should be

        *(unsigned long *)arg = onlined_pages;

or the initialization should be

        unsigned long onlined_pages = 0;



This is becoming a tad tiresome and I'd prefer not to have to check up
on such things.  Can we please get this right?

Sorry about that. Will fix it.

Regards,
Arun

Reply via email to