On 2/4/25 21:16, Peter Xu wrote:
On Tue, Feb 04, 2025 at 07:55:52PM +0100, David Hildenbrand wrote:
Ah, and now I remember where these 3 patches originate from: virtio-mem
handling.
For virtio-mem I want to register also a remap handler, for example, to
perform the custom preallocation handling.
So there will be at least two instances getting notified (memory backend,
virtio-mem), and the per-ramblock one would have only allowed to trigger one
(at least with a simple callback as we have today for ->resize).
I see, we can put something into commit log with such on decisions, then
we'll remember.
Said that, this still sounds like a per-ramblock thing, so instead of one
hook function we can also have per-ramblock notifier lists.
But I agree the perf issue isn't some immediate concern, so I'll leave that
to you and William. If so I think we should discuss that in the commit log
too, so we decide to not care about perf until necessary (or we just make
it per-ramblock..).
Thanks,
I agree that we could split this fix in 2 parts: The one fixing the
hugetlbfs (ignoring the preallocation setting for the moment), and the
notification mechanism as a second set of patches.
The first part would be the 3 first patches (including a corrected
version of patch 2) and the second part could be an adaptation of the
next 3 patches, with their notification implementation dealing with
merging, dump *and* preallocation setup.
But I'd be happy to help with the implementation of this 2nd aspect too:
In order to apply settings like preallocation to a RAMBLock we need to
find its associated HostMemoryBackend (where we have the 'prealloc' flag).
To do so, we record a RAMBlockNotifier in the HostMemoryBackend struct,
so that the notification triggered by the remap action:
ram_block_notify_remap(block->host, offset, page_size);
will go through the list of notifiers ram_list.ramblock_notifiers to run
the not NULL ram_block_remapped entries on all of them.
For each of them, we know the associated HostMemoryBackend (as it
contains the RAMBlockNotifier), and we verify which one corresponds to
the host address given, so that we can apply the appropriate settings.
IIUC, my proposal (with David's code) currently has a
per-HostMemoryBackend notification.
Now if I want to implement a per-RAMBlock notification, would you
suggest to consider that the 'mr' attibute of a RAMBlock always points
to a HostMemoryBackend.mr, so that we could get the HostMemoryBackend
associated to the block from a
container_of(block->mr, HostMemoryBackend, mr) ?
If this is valid, than we could apply the appropriate settings from
there, but can't we have RAMBlocks not pointing to a HostMemoryBackend.mr ?
I'm probably confused about what you are referring to.
So how would you suggest that I make the notification per-ramblock ?
Thanks in advance for your feedback.
I'll send a corrected version of the first 3 patches, unless you want to
go with the current version of the patches 4/6, 5/6 and 6/6, so that we
can deal with preallocation.
Please let me know.
Thanks,
William.
