[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 01/12] memory: Introduce RamDiscardMgr for RAM memory regi

From: Paolo Bonzini
Subject: Re: [PATCH v6 01/12] memory: Introduce RamDiscardMgr for RAM memory regions
Date: Mon, 22 Feb 2021 18:37:01 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 22/02/21 15:53, David Hildenbrand wrote:
Yes, but does it behave more like the IOMMU notifier in other regards?
:)  The IOMMU notifier is concerned with an iova concept that doesn't
exist at the MemoryRegion level, while RamDiscardListener works at the
(MemoryRegion, offset) level that can easily be represented by a
MemoryRegionSection.  Using MemoryRegionSection might even simplify the
listener code.

It's similarly concerned with rather small, lightweight updates I would say.

Why does that matter? I think if it's concerned with the MemoryRegion address space it should use MemoryListener and MemoryRegionSection.

The main motivation is to let listener decide how it wants to handle the
memory region. For example, for vhost, vdpa, kvm, ... I only want a
single region, not separate ones for each and every populated range,
punching out discarded ranges. Note that there are cases (i.e.,
anonymous memory), where it's even valid for the guest to read discarded

Yes, I agree with that.  You would still have the same
region-add/region_nop/region_del callbacks for KVM and friends; on top
of that you would have region_populate/region_discard callbacks for VFIO.

I think instead of region_populate/region_discard we would want individual region_add/region_del when populating/discarding for all MemoryListeners that opt-in somehow (e.g., VFIO, dump-guest-memory, ...). Similarly, we would want to call log_sync()/log_clear() then only for these parts.

But what happens when I populate/discard some memory? I don't want to trigger an address space transaction (begin()...region_nop()...commit()) - whenever I populate/discard memory (e.g., in 2 MB granularity). Especially not, if nothing might have changed for most other MemoryListeners.

Right, that was the reason why I was suggesting different callbacks. For the VFIO listener, which doesn't have begin or commit callbacks, I think you could just rename region_add to region_populate, and point both region_del and region_discard to the existing region_del commit.

Calling log_sync/log_clear only for populated parts also makes sense. log_sync and log_clear do not have to be within begin/commit, so you can change the semantics to call them more than once.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]