Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1624538

Re: your mail

Show all headers | View raw

On Sat, Apr 15, 2017 at 02:17:31PM +0200, Michal Hocko wrote:
> Hi,
> here I 3 more preparatory patches which I meant to send on Thursday but
> forgot... After more thinking about pfn walkers I have realized that
> the current code doesn't check offline holes in zones. From a quick
> review that doesn't seem to be a problem currently. Pfn walkers can race
> with memory offlining and with the original hotplug impementation those
> offline pages can change the zone but I wasn't able to find any serious
> problem other than small confusion. The new hotplug code, will not have
> any valid zone, though so those code paths should check PageReserved
> to rule offline holes. I hope I have addressed all of them in these 3
> patches. I would appreciate if Vlastimil and Jonsoo double check after
> me.

Hello, Michal.

s/Jonsoo/Joonsoo. :)

I'm not sure that it's a good idea to add PageResereved() check in pfn
walkers. First, this makes struct page validity check as two steps,
pfn_valid() and then PageResereved(). If we should not use struct page
in this case, it's better to pfn_valid() returns false rather than
adding a separate check. Anyway, we need to fix more places (all pfn
walker?) if we want to check validity by two steps.

The other problem I found is that your change will makes some
contiguous zones to be considered as non-contiguous. Memory allocated
by memblock API is also marked as PageResereved. If we consider this as
a hole, we will set such a zone as non-contiguous.

And, I guess that it's not enough to check PageResereved() in
pageblock_pfn_to_page() in order to skip these pages in compaction. If
holes are in the middle of the pageblock, pageblock_pfn_to_page()
cannot catch it and compaction will use struct page for this hole.

Therefore, I think that making pfn_valid() return false for not
onlined memory is a better solution for this problem. I don't know the
implementation detail for hotplug and I don't see your recent change
but we may defer memmap initialization until the zone is determined.
It will make pfn_valid() return false for un-initialized range.

Thanks.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

(none) Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
  [PATCH 2/3] mm, compaction: skip over holes in __reset_isolation_suitable Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
  Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-17 07:50 +0200
    Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-17 10:20 +0200
      Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-20 03:30 +0200
        Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 09:30 +0200
          Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 10:50 +0200
            Re: your mail Vlastimil Babka <vbabka@suse.cz> - 2017-04-20 14:00 +0200
              Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 14:20 +0200
          Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-21 06:40 +0200
            Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-21 09:20 +0200
              Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-24 03:50 +0200
                Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-24 10:00 +0200

csiph-web