On 11.09.19 11:23, Michael S. Tsirkin wrote: > On Tue, Sep 10, 2019 at 06:22:37PM +0200, David Hildenbrand wrote: >> On 10.09.19 18:18, Dr. David Alan Gilbert wrote: >>> * Alexander Duyck (alexander.duyck@xxxxxxxxx) wrote: >>>> On Tue, Sep 10, 2019 at 7:47 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote: >>>>> >>>>> On Tue 10-09-19 07:42:43, Alexander Duyck wrote: >>>>>> On Tue, Sep 10, 2019 at 5:42 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote: >>>>>>> >>>>>>> I wanted to review "mm: Introduce Reported pages" just realize that I >>>>>>> have no clue on what is going on so returned to the cover and it didn't >>>>>>> really help much. I am completely unfamiliar with virtio so please bear >>>>>>> with me. >>>>>>> >>>>>>> On Sat 07-09-19 10:25:03, Alexander Duyck wrote: >>>>>>> [...] >>>>>>>> This series provides an asynchronous means of reporting to a hypervisor >>>>>>>> that a guest page is no longer in use and can have the data associated >>>>>>>> with it dropped. To do this I have implemented functionality that allows >>>>>>>> for what I am referring to as unused page reporting >>>>>>>> >>>>>>>> The functionality for this is fairly simple. When enabled it will allocate >>>>>>>> statistics to track the number of reported pages in a given free area. >>>>>>>> When the number of free pages exceeds this value plus a high water value, >>>>>>>> currently 32, it will begin performing page reporting which consists of >>>>>>>> pulling pages off of free list and placing them into a scatter list. The >>>>>>>> scatterlist is then given to the page reporting device and it will perform >>>>>>>> the required action to make the pages "reported", in the case of >>>>>>>> virtio-balloon this results in the pages being madvised as MADV_DONTNEED >>>>>>>> and as such they are forced out of the guest. After this they are placed >>>>>>>> back on the free list, >>>>>>> >>>>>>> And here I am reallly lost because "forced out of the guest" makes me >>>>>>> feel that those pages are no longer usable by the guest. So how come you >>>>>>> can add them back to the free list. I suspect understanding this part >>>>>>> will allow me to understand why we have to mark those pages and prevent >>>>>>> merging. >>>>>> >>>>>> Basically as the paragraph above mentions "forced out of the guest" >>>>>> really is just the hypervisor calling MADV_DONTNEED on the page in >>>>>> question. So the behavior is the same as any userspace application >>>>>> that calls MADV_DONTNEED where the contents are no longer accessible >>>>>> from userspace and attempting to access them will result in a fault >>>>>> and the page being populated with a zero fill on-demand page, or a >>>>>> copy of the file contents if the memory is file backed. >>>>> >>>>> As I've said I have no idea about virt so this doesn't really tell me >>>>> much. Does that mean that if somebody allocates such a page and tries to >>>>> access it then virt will handle a fault and bring it back? >>>> >>>> Actually I am probably describing too much as the MADV_DONTNEED is the >>>> hypervisor behavior in response to the virtio-balloon notification. A >>>> more thorough explanation of it can be found by just running "man >>>> madvise", probably best just to leave it at that since I am probably >>>> confusing things by describing hypervisor behavior in a kernel patch >>>> set. >>>> >>>> For the most part all the page reporting really does is provide a way >>>> to incrementally identify unused regions of memory in the buddy >>>> allocator. That in turn is used by virtio-balloon in a polling thread >>>> to report to the hypervisor what pages are not in use so that it can >>>> make a decision on what to do with the pages now that it knows they >>>> are unused. >>>> >>>> All this is providing is just a report and it is optional if the >>>> hypervisor will act on it or not. If the hypervisor takes some sort of >>>> action on the page, then the expectation is that the hypervisor will >>>> use some sort of mechanism such as a page fault to discover when the >>>> page is used again. >>> >>> OK, that's interestingly different (but OK) from some other schemes that >>> hav ebeen described which *require* the guest to somehow indicate the >>> page is in use before starting to use the page again. >>> >> >> virtio-balloon also has a mode where the guest would not have to >> indicate to the host before re-using a page. Only >> VIRTIO_BALLOON_F_MUST_TELL_HOST enforces this. So it's not completely new. > > VIRTIO_BALLOON_F_MUST_TELL_HOST is a bit different. > When it's not set, guest still must tell host about > pages in use, it just can batch these notifications > sending them possibly after page has been used. > So even with VIRTIO_BALLOON_F_MUST_TELL_HOST off you don't > skip the notification. > I don't think so VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */ commit bf50e69f63d21091e525185c3ae761412be0ba72 Author: Dave Hansen <dave@xxxxxxxxxxxxxxxxxx> Date: Thu Apr 7 10:43:25 2011 -0700 virtio balloon: kill tell-host-first logic [...] But, if the bit is _not_ set, we are under no obligation to reverse the order; we're under no obligation to do _anything_. As of now, qemu-kvm defines the bit, but doesn't set it. Old code simply told the hypervisor afterwards, but only for little performance gain. It is not strictly necessary. > From hypervisor point of view, this feature is very much like adding > page to the balloon and immediately taking it out of the balloon again, > just doing it in one operation. > > The main difference is the contents of the page, which matters > with poisoning: in that case hypervisor is expected to hand > back page with the poisoning content. Not so with regular > deflate where page contents is undefined. > > Well and also the new interface is optimized for large chunks > of memory since we'll likely be dealing with such. > >>> Dave >> >> >> -- >> >> Thanks, >> >> David / dhildenb -- Thanks, David / dhildenb