Re: [PATCH v25 QEMU 3/3] virtio-balloon: Replace free page hinting references to 'report' with 'hint'

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 24, 2020 at 06:01:02PM +0200, David Hildenbrand wrote:
> On 24.06.20 17:37, Michael S. Tsirkin wrote:
> > On Wed, Jun 24, 2020 at 05:28:59PM +0200, David Hildenbrand wrote:
> >>> So at the high level the idea was simple, we just clear the dirty bit
> >>> when page is hinted, unless we sent a new command since. Implementation
> >>> was reviewed by migration maintainers. If there's a consensus the code
> >>> is written so badly we can't maintain it, maybe we should remove it.
> >>> Which parts are unmaintainable in your eyes - migration or virtio ones?
> >>
> >> QEMU implementation without a propert virtio specification. I hope that
> >> we can *at least* finally document the expected behavior. Alex gave it a
> >> shot, and I was hoping that Wei could jump in to clarify, help move this
> >> forward ... after all he implemented (+designed?) the feature and the
> >> virtio interface.
> >>
> >>> Or maybe it's the general thing that interface was never specced
> >>> properly.
> >>
> >> Yes, a spec would be definitely a good starter ...
> >>
> >> [...]
> >>
> >>>>
> >>>> 1. If migration fails during RAM precopy, the guest will never receive a
> >>>> DONE notification. Probably easy to fix.
> >>>>
> >>>> 2. Unclear semantics. Alex tried to document what the actual semantics
> >>>> of hinted pages are.
> >>>
> >>> I'll reply to that now.
> >>>
> >>>> Assume the following in the guest to a previously
> >>>> hinted page
> >>>>
> >>>> /* page was hinted and is reused now */
> >>>> if (page[x] != Y)
> >>>> 	page[x] == Y;
> >>>> /* migration ends, we now run on the destination */
> >>>> BUG_ON(page[x] != Y);
> >>>> /* BUG, because the content chan
> >>>
> >>> The assumption hinting makes is that data in page is writtent to before it's used.
> >>>
> >>>
> >>>> A guest can observe that. And that could be a random driver that just
> >>>> allocated a page.
> >>>>
> >>>> (I *assume* in Linux we might catch that using kasan, but I am not 100%
> >>>> sure, also, the actual semantics to document are unclear - e.g., for
> >>>> other guests)
> >>>
> >>> I think it's basically simple: hinting means it's ok to
> >>> fill page with trash unless it has been modified since the command
> >>> ID supplied.
> >>
> >> Yeah, I quite dislike the semantics, especially, as they are different
> >> to well-know semantics as e.g., represent in MADV_FREE. Getting changed
> >> content when reading is really weird. But it seemed to be easier to
> >> implement (low hanging fruit) and nobody complained back then. Well, now
> >> we are stuck with it.
> >>
> >> [..]
> > 
> > The difference with MADV_FREE is
> > - asynchronous (using cmd id to synchronize)
> > - zero not guaranteed
> > 
> > right?
> 
> *looking into man page*, yes, when reading you either get the old
> content or zero.
> 
> (I remember that a re-read also makes the content stable, but looks like
> you really have to write to a page)
> 
> We should most probably do what Alex suggested and initialize pages (at
> least write a single byte) when leaking them from the shrinker in the
> guest while hinting is active, such that the content is stable for
> anybody to allocate and reuse a page.

Drivers ignore old content from slab though, so I don't really see
the point.

> -- 
> Thanks,
> 
> David / dhildenb