linux-kernel - Re: [PATCH v15 6/7] virtio-balloon: Add support for providing free page reports to host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 13 Dec 2019 08:37:01 -0800
From:   Alexander Duyck <alexander.h.duyck@...ux.intel.com>
To:     David Hildenbrand <david@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        kvm@...r.kernel.org, mst@...hat.com, linux-kernel@...r.kernel.org,
        willy@...radead.org, mhocko@...nel.org, linux-mm@...ck.org,
        akpm@...ux-foundation.org, mgorman@...hsingularity.net,
        vbabka@...e.cz
Cc:     yang.zhang.wz@...il.com, nitesh@...hat.com, konrad.wilk@...cle.com,
        pagupta@...hat.com, riel@...riel.com, lcapitulino@...hat.com,
        dave.hansen@...el.com, wei.w.wang@...el.com, aarcange@...hat.com,
        pbonzini@...hat.com, dan.j.williams@...el.com, osalvador@...e.de
Subject: Re: [PATCH v15 6/7] virtio-balloon: Add support for providing free
 page reports to host

On Fri, 2019-12-13 at 11:15 +0100, David Hildenbrand wrote:
> On 05.12.19 17:22, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
> > 
> > Add support for the page reporting feature provided by virtio-balloon.
> > Reporting differs from the regular balloon functionality in that is is
> > much less durable than a standard memory balloon. Instead of creating a
> > list of pages that cannot be accessed the pages are only inaccessible
> > while they are being indicated to the virtio interface. Once the
> > interface has acknowledged them they are placed back into their respective
> > free lists and are once again accessible by the guest system.
> > 
> > Unlike a standard balloon we don't inflate and deflate the pages. Instead
> > we perform the reporting, and once the reporting is completed it is
> > assumed that the page has been dropped from the guest and will be faulted
> > back in the next time the page is accessed.
> > 
> > For this reason when I had originally introduced the patch set I referred
> > to this behavior as a "bubble" instead of a "balloon" since the duration
> > is short lived, and when the page is touched the "bubble" is popped and
> > the page is faulted back in.
> 
> While an interesting read, I would drop that comment as it isn't really
> of value for the code/codebase itself.

Okay, I can drop the comment.

> [...]
> 
> > +
> > +	vb->pr_dev_info.report = virtballoon_free_page_report;
> > +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> > +		unsigned int capacity;
> > +
> > +		capacity = virtqueue_get_vring_size(vb->reporting_vq);
> > +		if (capacity < PAGE_REPORTING_CAPACITY) {
> > +			err = -ENOSPC;
> > +			goto out_unregister_shrinker;
> 
> It's somewhat strange to fail loading the balloon completely here.
> Wouldn't it be better to print e.g. a warning but continue without free
> page reporting?
> 
> (I guess splitting up the list can be done in an addon patch if ever
> really needed for virtio-balloon)

That was kind of my thought. Odds are it probably is unlikely to come up.
At least with the code I have now I think the virtqueue size is something
like 128 and the capacity is only 32 as I wanted to limit the number of
pages that were being reported at the time.

> Apart from that
> 
> Reviewed-by: David Hildenbrand <david@...hat.com>
> 
> Thanks!
> 


Thanks for the review.

- Alex