Re: [PATCHv5 17/19] x86/mm: Implement sync_direct_mapping()

[Kirill A. Shutemov]

On Wed, Jul 18, 2018 at 05:01:37PM -0700, Dave Hansen wrote:
> On 07/17/2018 04:20 AM, Kirill A. Shutemov wrote:
> >  arch/x86/include/asm/mktme.h |   8 +
> >  arch/x86/mm/init_64.c        |  10 +
> >  arch/x86/mm/mktme.c          | 437 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 455 insertions(+)
> 
> I'm not the maintainer.  But, NAK from me on this on the diffstat alone.
> 
> There is simply too much technical debt here.  There is no way this code
> is not riddled with bugs and I would bet lots of beer on the fact that
> this has received little to know testing with all the combinations that
> matter, like memory hotplug.  I'd love to be proven wrong, so I eagerly
> await to be dazzled with the test results that have so far escaped
> mention in the changelog.
> 
> Please make an effort to refactor this to reuse the code that we already
> have to manage the direct mapping.  We can't afford 455 new lines of
> page table manipulation that nobody tests or runs.

I'll look in this once again. But I'm not sure that there's any better
solution.

The problem boils down to page allocation issue. We are not be able to
allocate enough page tables in early boot for all direct mappings. At that
stage we have very limited pool of pages that can be used for page tables.
The pool is allocated at compile-time and it's not enough to handle MKTME.

Syncing approach appeared to be the simplest to me.

Other possibility I see is to write down a journal of operations on direct
mappings to be replayed once we have proper page allocator around.

> How _was_ this tested?

Besides normal boot with MTKME enabled and access pages via new direct
mappings, I also test memory hotplug and hotremove with QEMU.

Ideally we wound need some self-test for this. But I don't see a way to
simulate hotplug and hotremove. Soft offlining doesn't cut it. We
actually need to see the ACPI event to trigger the code.

-- 
 Kirill A. Shutemov