On Thu, Jul 29, 2021 at 06:15:58PM +0200, David Hildenbrand wrote: > On 29.07.21 17:52, Peter Xu wrote: > > On Thu, Jul 29, 2021 at 02:14:41PM +0200, David Hildenbrand wrote: > > > On 24.07.21 00:10, Peter Xu wrote: > > > > On Fri, Jul 23, 2021 at 09:01:42PM +0200, David Hildenbrand wrote: > > > > > It can happen in corner cases and is valid: with the current > > > > > virtio-mem > > > > > spec, guests are allowed to read unplugged memory. This will, for > > > > > example, > > > > > happen on older Linux guests when reading /proc/kcore or (with even > > > > > older > > > > > guests) when dumping guest memory via kdump. These corner cases were > > > > > the > > > > > main reason why the spec allows for it -- until we have guests > > > > > properly > > > > > adjusted such that it won't happen even in corner cases. > > > > > > > > > > A future feature bit will disallow it for the guest: required for > > > > > supporting > > > > > shmem/hugetlb cleanly. With that in place, I agree that we would want > > > > > to > > > > > warn in this case! > > > > > > > > OK that makes sense; with the page_size change, feel free to add: > > > > > > I just realized that relying on the page_size would be wrong. > > > > > > We migrate TARGET_PAGE_SIZE chunks and the offset might not be page_size > > > aligned. So if we were to replace TARGET_PAGE_SIZE by rb->page_size, we > > > might accidentally cover a "too big" range. > > > > I'm wondering whether we should make the offset page size aligned instead. > > For > > example, note that postcopy_place_page_zero() should only take page_size > > aligned host addr or UFFDIO_COPY could fail (hugetlb doesn't support > > UFFDIO_ZEROPAGE yet). > > That is true indeed. I'd assume in that case that we would get called with > the proper offset already, right? Because uffd would only report properly > aligned pages IIRC.
Nop; it should return the faulted address. So postcopy_request_page() may need some alignment work, as it was handled in migrate_send_rp_req_pages(). > > > > > Btw, does virtio-mem supports hugetlbfs now? When with it, the smallest > > unit > > to plug/unplug would the huge page size (e.g., for 1g huge page, sounds not > > helpful to unplug 2M memory), am I right? > > It supports it to 99.9 % I'd say (especially with the dump/tpm/migration > fixes upstream). The units are handled properly: the unit is at least as big > as the page size. > > So with 1 GiB pages, you have a unit of 1 GiB. I see, thanks for confirming. Then fixing up the offset seems to be the right thing to do. Thanks, -- Peter Xu
