Re: [RFC RESEND 0/6] hugetlbfs largepage RAS project

[David Hildenbrand <david@redhat.com>]

Hi again,

>>> This is a Qemu RFC to introduce the possibility to deal with hardware
>>> memory errors impacting hugetlbfs memory backed VMs. When using
>>> hugetlbfs large pages, any large page location being impacted by an
>>> HW memory error results in poisoning the entire page, suddenly making
>>> a large chunk of the VM memory unusable.
>>>
>>> The implemented proposal is simply a memory mapping change when an HW
>>> error
>>> is reported to Qemu, to transform a hugetlbfs large page into a set of
>>> standard sized pages. The failed large page is unmapped and a set of
>>> standard sized pages are mapped in place.
>>> This mechanism is triggered when a SIGBUS/MCE_MCEERR_Ax signal is
>>> received
>>> by qemu and the reported location corresponds to a large page.

One clarifying question: you simply replace the hugetlb page by multiple 
small pages using mmap(MAP_FIXED). So you

(a) are not able to recover any memory of the original page (as of now)
(b) no longer have a hugetlb page and, therefore, possibly a performance
     degradation, relevant in low-latency applications that really care
     about the usage of hugetlb pages.
(c) run into the described inconsistency issues

Why is what you propose beneficial over just fallocate(PUNCH_HOLE) the 
full page and get a fresh, non-poisoned page instead?

Sure, you have to reserve some pages if that ever happens, but what is 
the big selling point over PUNCH_HOLE + realloc? (sorry if I missed it 
and it was spelled out)

>>>
>>> This gives the possibility to:
>>> - Take advantage of newer hypervisor kernel providing a way to retrieve
>>> still valid data on the impacted hugetlbfs poisoned large page.

Reading that again, that shouldn't have to be hypervisor-specific. 
Really, if someone were to extract data from a poisoned hugetlb folio, 
it shouldn't be hypervisor-specific. The kernel should be able to know 
which regions are accessible and could allow ways for reading these, one 
way or the other.

It could just be a fairly hugetlb-special feature that would replace the 
poisoned page by a fresh hugetlb page where as much page content as 
possible has been recoverd from the old one.

>>> If the backend file is MAP_SHARED, we can copy the valid data into the
> 
> 
> Thank you David for this first reaction on this proposal.
> 
> 
>> How are you dealing with other consumers of the shared memory,
>> such as vhost-user processes,
> 
> 
> In the current proposal, I don't deal with this aspect.
> In fact, any other process sharing the changed memory will
> continue to map the poisoned large page. So any access to
> this page will generate a SIGBUS to this other process.
> 
> In this situation vhost-user processes should continue to receive
> SIGBUS signals (and probably continue to die because of that).

That's ... suboptimal. :)

Assume you have a 1 GiB page. The guest OS can happily allocate buffers 
in there so they can end up in vhost-user and crash that process. 
Without any warning.

> 
> So I do see a real problem if 2 qemu processes are sharing the
> same hugetlbfs segment -- in this case, error recovery should not
> occur on this piece of the memory. Maybe dealing with this situation
> with "ivshmem" options is doable (marking the shared segment
> "not eligible" to hugetlbfs recovery, just like not "share=on"
> hugetlbfs entries are not eligible)
> -- I need to think about this specific case.
> 
> Please let me know if there is a better way to deal with this
> shared memory aspect and have a better system reaction.

Not creating the inconsistency in the first place :)

>> vm migration whereby RAM is migrated using file content,
> 
> 
> Migration doesn't currently work with memory poisoning.
> You can give a look at the already integrated following commit:
> 
> 06152b89db64 migration: prevent migration when VM has poisoned memory
> 
> This proposal doesn't change anything on this side.

That commit is fairly fresh and likely missed the option to *not* 
migrate RAM by reading it, but instead by migrating it through a shared 
file. For example, VM life-upgrade (CPR) wants to use that (or is 
already using that), to avoid RAM migration completely.

> 
>> vfio that might have these pages pinned?
> 
> AFAIK even pinned memory can be impacted by memory error and poisoned
> by the kernel. Now as I said in the cover letter, I'd like to know if
> we should take extra care for IO memory, vfio configured memory buffers...

Assume your GPU has a hugetlb folio pinned via vfio. As soon as you make 
the guest RAM point at anything else as VFIO is aware of, we end up in 
the same problem we had when we learned about having to disable balloon 
inflation (MADVISE_DONTNEED) as soon as VFIO pinned pages.

We'd have to inform VFIO that the mapping is now different. Otherwise 
it's really better to crash the VM than having your GPU read/write 
different data than your CPU reads/writes,

> 
> 
>> In general, you cannot simply replace pages by private copies
>> when somebody else might be relying on these pages to go to
>> actual guest RAM.
> 
> This is correct, but the current proposal is dealing with a specific
> shared memory type: poisoned large pages. So any other process mapping
> this type of page can't access it without generating a SIGBUS.

Right, and that's the issue. Because, for example, how should the VM be 
aware that this memory is now special and must not be used for some 
purposes without leading to problems elsewhere?

> 
> 
>> It sounds very hacky and incomplete at first.
> 
> As you can see, RAS features need to be completed.
> And if this proposal is incomplete, what other changes should be
> done to complete it ?
> 
> I do hope we can discuss this RFC to adapt what is incorrect, or
> find a better way to address this situation.

One long-term goal people are working on is to allow remapping the 
hugetlb folios in smaller granularity, such that only a single affected 
PTE can be marked as poisoned. (used to be called high-granularity-mapping)

However, at the same time, the focus hseems to shift towards using 
guest_memfd instead of hugetlb, once it supports 1 GiB pages and shared 
memory. It will likely be easier to support mapping 1 GiB pages using 
PTEs that way, and there are ongoing discussions how that can be 
achieved more easily.

There are also discussions [1] about not poisoning the mappings at all 
and handling it differently. But I haven't yet digested how exactly that 
could look like in reality.


[1] https://lkml.kernel.org/r/20240828234958.GE3773488@nvidia.com

-- 
Cheers,

David / dhildenb