linux-kernel - Re: [PATCH v1 3/4] mm/memory_hotplug: Simplify online_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <97885e6d-7b54-e3a5-dbca-67d0d19abc02@redhat.com>
Date:   Wed, 14 Aug 2019 16:23:09 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Oscar Salvador <osalvador@...e.de>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH v1 3/4] mm/memory_hotplug: Simplify online_pages_range()

On 14.08.19 16:19, Michal Hocko wrote:
> On Fri 09-08-19 14:57:00, David Hildenbrand wrote:
>> move_pfn_range_to_zone() will set all pages to PG_reserved via
>> memmap_init_zone(). The only way a page could no longer be reserved
>> would be if a MEM_GOING_ONLINE notifier would clear PG_reserved - which
>> is not done (the online_page callback is used for that purpose by
>> e.g., Hyper-V instead). walk_system_ram_range() will never call
>> online_pages_range() with duplicate PFNs, so drop the PageReserved() check.
>>
>> Simplify the handling, as online_pages always corresponds to nr_pages.
>> There is no need for online_pages_blocks() anymore.
> 
> This would be easier to review if split up into two patches. One that
> only performs cleanup without any other changes and the PageReserved
> check. I like the check going away and we should get rid of the
> dependency on the Reserved bit completely.

Yes, makes sense.

> 
> Other than that I find the start_pfn and pfn being used both for
> iteration each a different way really confusing and I cannot convince
> myself it is even correct because I didn't bother to look deeper as
> I simply think that the order manipulation from the previous is just
> making things worse at this moment. If the problem is even real then it
> can be done on top instead with some real example of the memory layout
> that breaks.

Yes, I'll rework this. Only "pfn" should be used in the loop (it seems
to be correct but confusing I agree).

-- 

Thanks,

David / dhildenb