Linux Virtualization - Jun 2018 - [PULL] vhost: cleanups and fixes

The following changes since commit 29dcea88779c856c7dc92040a0c01233263101d4:

  Linux 4.17 (2018-06-03 14:15:21 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to aa15783ee62d57d69433101ede3e3ed11e48161d:

  virtio: update the comments for transport features (2018-06-07 22:17:40 +0300)

----------------------------------------------------------------
virtio, vhost: features, fixes

VF support for virtio.
Free page hint request support for VM migration.
DMA barriers for virtio strong barriers.
Bugfixes.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>

----------------------------------------------------------------
Michael S. Tsirkin (2):
      virtio_ring: switch to dma_XX barriers for rpmsg
      vhost: fix info leak due to uninitialized memory

Tiwei Bie (2):
      virtio_pci: support enabling VFs
      virtio: update the comments for transport features

Wei Wang (4):
      mm: support reporting free page blocks
      virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
      mm/page_poison: expose page_poisoning_enabled to kernel modules
      virtio-balloon: VIRTIO_BALLOON_F_PAGE_POISON

 drivers/vhost/vhost.c               |   3 +
 drivers/virtio/virtio_balloon.c     | 298 +++++++++++++++++++++++++++++++-----
 drivers/virtio/virtio_pci_common.c  |  30 ++++
 drivers/virtio/virtio_pci_modern.c  |  14 ++
 include/linux/mm.h                  |   6 +
 include/linux/virtio_ring.h         |   4 +-
 include/uapi/linux/virtio_balloon.h |   7 +
 include/uapi/linux/virtio_config.h  |  16 +-
 mm/page_alloc.c                     |  97 ++++++++++++
 mm/page_poison.c                    |   6 +
 10 files changed, 439 insertions(+), 42 deletions(-)

On Mon, Jun 11, 2018 at 9:24 AM Michael S. Tsirkin <mst at redhat.com>
wrote:
>
>       virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
Is this really a good idea?

Plus it seems entirely broken.

The report_pfn_range() callback is done under the zone lock, but
virtio_balloon_send_free_pages() (which is the only callback used that
I can find) does add_one_sg(), which does virtqueue_add_inbuf(vq, &sg,
1, vq, GFP_KERNEL);

So now we apparently do a GFP_KERNEL allocation insider the mm zone
lock, which is broken on just _so_ many levels.

Pulled and then unpulled again.

Either somebody needs to explain why I'm wrong and you can re-submit
this, or this kind of garbage needs to go away.

I do *not* want to be in the situation where I pull stuff from the
virtio people that adds completely broken core VM functionality.

Because if I'm in that situation, I will stop pulling from you guys.
Seriously. You have *no* place sending me broken shit that is outside
the virtio layer.

Subsystems that break code MM will get shunned. You just aren't
important enough to allow you breaking code VM.

                Linus

On Mon, Jun 11, 2018 at 11:32 AM Linus Torvalds
<torvalds at linux-foundation.org> wrote:
>
> So now we apparently do a GFP_KERNEL allocation insider the mm zone
> lock, which is broken on just _so_ many levels.
Oh, I see the comment about how it doesn't actually do an allocation
at all because it's a single-entry.

Still too damn ugly to live, and much too fragile. No way in hell do
we even _hint_ at a GFP_KERNEL when we're inside a context that can't
do any memory allocation at all.

Plus I'm not convinced it's a "no allocation" path even
despite that
comment, because it also does a "dma_map_page()" etc, which can cause
allocations to do the dma mapping thing afaik. No?

Maybe there's some reason why that doesn't happen either, but
basically this whole callchain looks *way* to complicated to be used
under a core VM spinlock.

                Linus

On Mon, Jun 11, 2018 at 11:32:41AM -0700, Linus Torvalds
wrote:
> On Mon, Jun 11, 2018 at 9:24 AM Michael S. Tsirkin <mst at
redhat.com> wrote:
> >
> >       virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
> 
> Is this really a good idea?
Well knowing which pages are unused does seem to be useful.  Do you
refer to this generally or to the idea of scanning the free list,
or to the specific implementation issues you listed below?

> Plus it seems entirely broken.
> 
> The report_pfn_range() callback is done under the zone lock, but
> virtio_balloon_send_free_pages() (which is the only callback used that
> I can find) does add_one_sg(), which does virtqueue_add_inbuf(vq, &sg,
> 1, vq, GFP_KERNEL);
> 
> So now we apparently do a GFP_KERNEL allocation insider the mm zone
> lock, which is broken on just _so_ many levels.
> 
> Pulled and then unpulled again.
> 
> Either somebody needs to explain why I'm wrong and you can re-submit
> this, or this kind of garbage needs to go away.
> 
> I do *not* want to be in the situation where I pull stuff from the
> virtio people that adds completely broken core VM functionality.
> 
> Because if I'm in that situation, I will stop pulling from you guys.
> Seriously. You have *no* place sending me broken shit that is outside
> the virtio layer.
> 
> Subsystems that break code MM will get shunned. You just aren't
> important enough to allow you breaking code VM.
> 
>                 Linus
So no, it doesn't do any allocations - GFP_KERNEL or otherwise, but I
agree how it might be confusing.

So I'll drop this for now, but it would be nice if there is a bit more
direction on this feature since I know several people are trying to work
on this.


-- 
MST