>>> >>> The PAGE_POISON feature is being used to indicate that we expect the >>> page to contain a certain value when it is returned to us. With the >>> current implementation in QEMU if that value is zero we can work with >>> it because that is the effect that MADV_DONTNEED has. However if there >>> were some other effect being used by QEMU we would want to know that >>> the guest is expecting us to write a 0 page. So I believe it makes >>> sense to inform the hypervisor that we are doing something that >>> expects us to fill the page with zeros in the case of >> >> Informing makes sense, yes. And we inform it via the poison feature, if >> possible. This discussion is about "what happens if the poison feature >> is not there, but we do have want_init_on_free()." > > My preference is to err on the side of caution. I view want_init_on > free() as equivalent to page_poison_enabled with > CONFIG_PAGE_POISONING_ZERO and treated the same way. So if we are > going to let one by then we would want to let the other by as well. > Agreed. >>> want_init_on_free rather than not providing that information to QEMU. >>> This way, if in the future we decide to change the implementation in >>> some way that might effect the value contained in the pages, we might >> >> If the hypervisor can no longer guarantee "either the old page, or a >> page filled with zero", it would have to disable the feature. I don't >> see that happening, really. > > My concern is we can never see how things might be used in the future. > The advantage of setting the poison feature flag and leaving the value > as 0 is that it indicates that we are strictly expecting a zero page > versus allowing the page to contain some other content. I'm expecting > that in most cases that we should see that the page poisoning feature > will be present if page reporting is present so my thought is it is > safer to put the limitation on the guest rather than on the host as it > allows us more flexibility for future implementations. > I consider it very unlikely that you want to provide something else than - the old page - a zeroed page But yes, while having VIRTIO_BALLOON_F_REPORTING && !VIRTIO_BALLOON_F_PAGE_POISON in QEMU as of now would be possible, it's unlikely that we'll see this very often. Similar to want_init_on_free() >>> have the flexibility to identify the want_init_on_free case so we can >>> work around it. >> >> I don't have a too strong opinion here, but this sounds like one of the >> improvements we wanted to have for free page hinting, but learned that >> it's not possible because it was *underspecified*. > > Right. For free page hinting it is underspecified. By defining a tight > relationship between free page reporting and page poison we might be > able to better handle those sorts of cases. I suspect there may be > interest in having free page reporting eventually take on many of the > responsibilities of free page hinting as it doesn't have the extra > complication of having to consume all of the guests memory. If we > dictate that free page reporting has to have a very clear definition > in relation to any page initialization it lays the groundwork for us > to be able to eventually handle those kind of cases. I would encourage you to document the semantics of free page reporting properly - especially, that we decided that the content of a reported page with VIRTIO_BALLOON_F_REPORTING && !VIRTIO_BALLOON_F_PAGE_POISON is *undefined* and needs a reinit. Only with VIRTIO_BALLOON_F_REPORTING && VIRTIO_BALLOON_F_PAGE_POISON the content is either the old page content or a page filled with poison_val. So take my Acked-by: David Hildenbrand <david@xxxxxxxxxx> -- Thanks, David / dhildenb _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization