David Hildenbrand
2017-Jun-20 18:54 UTC
[PATCH v11 4/6] mm: function to offer a page block on the free list
On 20.06.2017 20:17, Michael S. Tsirkin wrote:> On Tue, Jun 20, 2017 at 06:49:33PM +0200, David Hildenbrand wrote: >> On 20.06.2017 18:44, Rik van Riel wrote: >>> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote: >>> >>>> The hypervisor is going to throw away the contents of these pages, >>>> right? As soon as the spinlock is released, someone can allocate a >>>> page, and put good data in it. What keeps the hypervisor from >>>> throwing >>>> away good data? >>> >>> That looks like it may be the wrong API, then? >>> >>> We already have hooks called arch_free_page and >>> arch_alloc_page in the VM, which are called when >>> pages are freed, and allocated, respectively. >>> >>> Nitesh Lal (on the CC list) is working on a way >>> to efficiently batch recently freed pages for >>> free page hinting to the hypervisor. >>> >>> If that is done efficiently enough (eg. with >>> MADV_FREE on the hypervisor side for lazy freeing, >>> and lazy later re-use of the pages), do we still >>> need the harder to use batch interface from this >>> patch? >>> >> David's opinion incoming: >> >> No, I think proper free page hinting would be the optimum solution, if >> done right. This would avoid the batch interface and even turn >> virtio-balloon in some sense useless. > > I agree generally. But we have to balance that against the fact that > this was discussed since at least 2011 and no one built this solution > yet.I totally agree, and I still think it will be hard to get a decent performance for free page hinting (let's call it challenging). But I heard of some interesting ideas. Surprise me. Still, I would favor such an interface over a mm interface where people start asking the same question over and over again ("how can this even work"). Not only because it wasn't explained sufficiently enough, but also because this interface is so special for one use case and one scenario (concurrent dirty tracking in the host during migration). IMHO even simply writing all-zeros to all free pages before starting migration (or even when freeing a page) would be a cleaner interface than this (because it atomically works with the entity the host cares about for migration). But yes, performance is horrible that's why I am not even suggesting it. Just saying that this mm interface is very very special and if we could find something better, I'd favor it. -- Thanks, David
Michael S. Tsirkin
2017-Jun-20 18:56 UTC
[PATCH v11 4/6] mm: function to offer a page block on the free list
On Tue, Jun 20, 2017 at 08:54:29PM +0200, David Hildenbrand wrote:> On 20.06.2017 20:17, Michael S. Tsirkin wrote: > > On Tue, Jun 20, 2017 at 06:49:33PM +0200, David Hildenbrand wrote: > >> On 20.06.2017 18:44, Rik van Riel wrote: > >>> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote: > >>> > >>>> The hypervisor is going to throw away the contents of these pages, > >>>> right? As soon as the spinlock is released, someone can allocate a > >>>> page, and put good data in it. What keeps the hypervisor from > >>>> throwing > >>>> away good data? > >>> > >>> That looks like it may be the wrong API, then? > >>> > >>> We already have hooks called arch_free_page and > >>> arch_alloc_page in the VM, which are called when > >>> pages are freed, and allocated, respectively. > >>> > >>> Nitesh Lal (on the CC list) is working on a way > >>> to efficiently batch recently freed pages for > >>> free page hinting to the hypervisor. > >>> > >>> If that is done efficiently enough (eg. with > >>> MADV_FREE on the hypervisor side for lazy freeing, > >>> and lazy later re-use of the pages), do we still > >>> need the harder to use batch interface from this > >>> patch? > >>> > >> David's opinion incoming: > >> > >> No, I think proper free page hinting would be the optimum solution, if > >> done right. This would avoid the batch interface and even turn > >> virtio-balloon in some sense useless. > > > > I agree generally. But we have to balance that against the fact that > > this was discussed since at least 2011 and no one built this solution > > yet. > > I totally agree, and I still think it will be hard to get a decent > performance for free page hinting (let's call it challenging). But I > heard of some interesting ideas. Surprise me. > > Still, I would favor such an interface over a mm interface where people > start asking the same question over and over again ("how can this even > work"). Not only because it wasn't explained sufficiently enough, but > also because this interface is so special for one use case and one > scenario (concurrent dirty tracking in the host during migration). > > IMHO even simply writing all-zeros to all free pages before starting > migration (or even when freeing a page) would be a cleaner interface > than this (because it atomically works with the entity the host cares > about for migration). But yes, performance is horrible that's why I am > not even suggesting it. Just saying that this mm interface is very very > special and if we could find something better, I'd favor it.As long as there's a single user, changing to a better interface once it's found won't be hard at all :)> -- > > Thanks, > > David
David Hildenbrand
2017-Jun-20 19:01 UTC
[PATCH v11 4/6] mm: function to offer a page block on the free list
>> IMHO even simply writing all-zeros to all free pages before starting >> migration (or even when freeing a page) would be a cleaner interface >> than this (because it atomically works with the entity the host cares >> about for migration). But yes, performance is horrible that's why I am >> not even suggesting it. Just saying that this mm interface is very very >> special and if we could find something better, I'd favor it. > > As long as there's a single user, changing to a better interface > once it's found won't be hard at all :) >Hehe, more like "we made this beautiful virtio-balloon extension" - oh there is free page hinting (assuming that it does not reuse the batch interface here). Guess how long it would take to at least show that free page hinting can be done. If it takes another 6 years, I am totally on your side ;) -- Thanks, David
Seemingly Similar Threads
- [PATCH v11 4/6] mm: function to offer a page block on the free list
- [PATCH v11 4/6] mm: function to offer a page block on the free list
- [PATCH v11 4/6] mm: function to offer a page block on the free list
- [PATCH v11 4/6] mm: function to offer a page block on the free list
- [PATCH v11 4/6] mm: function to offer a page block on the free list