| Message ID | 20190214043916.22128-1-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| Headers | show |
| Series | Improve balloon handling of pagesizes other than 4kiB | expand |
On Thu, Feb 14, 2019 at 03:39:11PM +1100, David Gibson wrote: > I posted some RFCs for this back in December, but didn't wrap it up in > time for 3.1. Posting again for inclusion in 4.0. > > The virtio-balloon devices was never really thought out for cases > other than 4kiB pagesize on both guest and host. It works in some > cases, but in others can be ineffectual or even cause guest memory > corruption. > > This series makes a handful of preliminary cleanups, then makes a > change to safely, though not perfectly, handle cases with non 4kiB > pagesizes. I'd like to see a version of this that does not depend on patch 1 which is not a cleanup nor a bugfix. Could you look into this please? We can then debate merits of patch 1 separately. > Changes since RFC: > * Further refinement of when to issue warnings in 5/5 > > David Gibson (5): > virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate > virtio-balloon: Corrections to address verification > virtio-balloon: Rework ballon_page() interface > virtio-balloon: Use ram_block_discard_range() instead of raw madvise() > virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size > > hw/virtio/virtio-balloon.c | 102 ++++++++++++++++++++++++----- > include/hw/virtio/virtio-balloon.h | 3 + > 2 files changed, 89 insertions(+), 16 deletions(-) > > -- > 2.20.1
On Thu, Feb 28, 2019 at 08:39:21AM -0500, Michael S. Tsirkin wrote: > On Thu, Feb 14, 2019 at 03:39:11PM +1100, David Gibson wrote: > > I posted some RFCs for this back in December, but didn't wrap it up in > > time for 3.1. Posting again for inclusion in 4.0. > > > > The virtio-balloon devices was never really thought out for cases > > other than 4kiB pagesize on both guest and host. It works in some > > cases, but in others can be ineffectual or even cause guest memory > > corruption. > > > > This series makes a handful of preliminary cleanups, then makes a > > change to safely, though not perfectly, handle cases with non 4kiB > > pagesizes. > > I'd like to see a version of this that does not depend on patch 1 which > is not a cleanup nor a bugfix. Could you look into this please? Ok... the original series is already applied to master, so I'm not exactly sure what you want me to do here. Should I try to come up with a "logical revert" of the first patch? Or do you intend to revert the whole series, and I rewrite the series without the first patch? > We can then debate merits of patch 1 separately. > > > > Changes since RFC: > > * Further refinement of when to issue warnings in 5/5 > > > > David Gibson (5): > > virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate > > virtio-balloon: Corrections to address verification > > virtio-balloon: Rework ballon_page() interface > > virtio-balloon: Use ram_block_discard_range() instead of raw madvise() > > virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size > > > > hw/virtio/virtio-balloon.c | 102 ++++++++++++++++++++++++----- > > include/hw/virtio/virtio-balloon.h | 3 + > > 2 files changed, 89 insertions(+), 16 deletions(-) > > >
On Tue, Mar 05, 2019 at 11:53:32AM +1100, David Gibson wrote: > On Thu, Feb 28, 2019 at 08:39:21AM -0500, Michael S. Tsirkin wrote: > > On Thu, Feb 14, 2019 at 03:39:11PM +1100, David Gibson wrote: > > > I posted some RFCs for this back in December, but didn't wrap it up in > > > time for 3.1. Posting again for inclusion in 4.0. > > > > > > The virtio-balloon devices was never really thought out for cases > > > other than 4kiB pagesize on both guest and host. It works in some > > > cases, but in others can be ineffectual or even cause guest memory > > > corruption. > > > > > > This series makes a handful of preliminary cleanups, then makes a > > > change to safely, though not perfectly, handle cases with non 4kiB > > > pagesizes. > > > > I'd like to see a version of this that does not depend on patch 1 which > > is not a cleanup nor a bugfix. Could you look into this please? > > Ok... the original series is already applied to master, so I'm not > exactly sure what you want me to do here. Should I try to come up > with a "logical revert" of the first patch? Or do you intend to > revert the whole series, and I rewrite the series without the first > patch? Whatever you prefer. Maybe the best idea is to add a flag that says whether to madvise or not. Default can be compatible. Hmm? > > We can then debate merits of patch 1 separately. > > > > > > > Changes since RFC: > > > * Further refinement of when to issue warnings in 5/5 > > > > > > David Gibson (5): > > > virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate > > > virtio-balloon: Corrections to address verification > > > virtio-balloon: Rework ballon_page() interface > > > virtio-balloon: Use ram_block_discard_range() instead of raw madvise() > > > virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size > > > > > > hw/virtio/virtio-balloon.c | 102 ++++++++++++++++++++++++----- > > > include/hw/virtio/virtio-balloon.h | 3 + > > > 2 files changed, 89 insertions(+), 16 deletions(-) > > > > > > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
On Mon, Mar 04, 2019 at 09:13:03PM -0500, Michael S. Tsirkin wrote: > On Tue, Mar 05, 2019 at 11:53:32AM +1100, David Gibson wrote: > > On Thu, Feb 28, 2019 at 08:39:21AM -0500, Michael S. Tsirkin wrote: > > > On Thu, Feb 14, 2019 at 03:39:11PM +1100, David Gibson wrote: > > > > I posted some RFCs for this back in December, but didn't wrap it up in > > > > time for 3.1. Posting again for inclusion in 4.0. > > > > > > > > The virtio-balloon devices was never really thought out for cases > > > > other than 4kiB pagesize on both guest and host. It works in some > > > > cases, but in others can be ineffectual or even cause guest memory > > > > corruption. > > > > > > > > This series makes a handful of preliminary cleanups, then makes a > > > > change to safely, though not perfectly, handle cases with non 4kiB > > > > pagesizes. > > > > > > I'd like to see a version of this that does not depend on patch 1 which > > > is not a cleanup nor a bugfix. Could you look into this please? > > > > Ok... the original series is already applied to master, so I'm not > > exactly sure what you want me to do here. Should I try to come up > > with a "logical revert" of the first patch? Or do you intend to > > revert the whole series, and I rewrite the series without the first > > patch? > > Whatever you prefer. Maybe the best idea is to add a flag > that says whether to madvise or not. Default can be compatible. > Hmm? Ok, will do. As I was looking at this I noticed a bug in the current code. Extremely unlikely to hit in practice, but could result in guest memory corruption if it does. So, I'll fix that as well.