On Fri, Jul 23, 2021 at 08:36:32PM +0200, David Hildenbrand wrote: > > > +static int postcopy_request_page(MigrationIncomingState *mis, RAMBlock > > > *rb, > > > + ram_addr_t start, uint64_t haddr) > > > +{ > > > + /* > > > + * Discarded pages (via RamDiscardManager) are never migrated. On > > > unlikely > > > + * access, place a zeropage, which will also set the relevant bits > > > in the > > > + * recv_bitmap accordingly, so we won't try placing a zeropage twice. > > > + */ > > > + if (ramblock_page_is_discarded(rb, start)) { > > > + bool received = ramblock_recv_bitmap_test_byte_offset(rb, start); > > > > Will received be set for any case with the current code base? As I thought > > virtio-mem forbids plug/unplug during the whole lifecycle of migration. > > receive would only be set if you have two CPUs faulting on the same address > at the same time and the first one already placed a zeropage on this code > path (as the comment said, that will implicitly set it in the rceivedmask).
Ah I see; or just ignore the error of postcopy_place_page_zero() here because per my understanding this whole path is not expected after all. > > So, pretty unlikely to happen, but if the stars align ... :) > > > > > > + > > > + return received ? 0 : postcopy_place_page_zero(mis, (void > > > *)haddr, rb); > > > > (now we can fill up pages in two threads.. but looks thread-safe) > > > > Meanwhile if this is highly not wanted, maybe worth an error_report_once() > > so > > the admin could see something? > > > You mean, if postcopy_place_page_zero() fails? I meant ramblock_page_is_discarded() shouldn't really trigger for a sane guest, right? Because it means the guest is accessing some unplugged memory. I wanted to give a heads-up to the admin so he/she knows there's something odd. Also that can be a hint for debugging if it's hit for any unknown reasons. Thanks, -- Peter Xu