KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: David Hildenbrand <david@redhat.com>
Cc: Nitesh Narayan Lal <nitesh@redhat.com>,
	kvm list <kvm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	virtio-dev@lists.oasis-open.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	lcapitulino@redhat.com, Pankaj Gupta <pagupta@redhat.com>,
	"Wang, Wei W" <wei.w.wang@intel.com>,
	Yang Zhang <yang.zhang.wz@gmail.com>,
	Rik van Riel <riel@surriel.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	dodgen@google.com, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	dhildenb@redhat.com, Andrea Arcangeli <aarcange@redhat.com>,
	john.starks@microsoft.com, Dave Hansen <dave.hansen@intel.com>,
	Michal Hocko <mhocko@suse.com>,
	cohuck@redhat.com
Subject: Re: [RFC][Patch v12 1/2] mm: page_reporting: core infrastructure
Date: Tue, 13 Aug 2019 16:14:36 -0700
Message-ID: <CAKgT0UfaaHrEaS2wsbdTuzCdCtSrM4Tx79w=dP8HPEnq+T7rtQ@mail.gmail.com> (raw)
In-Reply-To: <32f61f87-6205-5001-866c-a84e20fc9d85@redhat.com>

On Tue, Aug 13, 2019 at 3:34 AM David Hildenbrand <david@redhat.com> wrote:
>
> >>>> +static int process_free_page(struct page *page,
> >>>> +                            struct page_reporting_config *phconf, int count)
> >>>> +{
> >>>> +       int mt, order, ret = 0;
> >>>> +
> >>>> +       mt = get_pageblock_migratetype(page);
> >>>> +       order = page_private(page);
> >>>> +       ret = __isolate_free_page(page, order);
> >>>> +
> >> I just started looking into the wonderful world of
> >> isolation/compaction/migration.
> >>
> >> I don't think saving/restoring the migratetype is correct here. AFAIK,
> >> MOVABLE/UNMOVABLE/RECLAIMABLE is just a hint, doesn't mean that e.g.,
> >> movable pages and up in UNMOVABLE or ordinary kernel allocations on
> >> MOVABLE. So that shouldn't be an issue - I guess.
> >>
> >> 1. You should never allocate something that is no
> >> MOVABLE/UNMOVABLE/RECLAIMABLE. Especially not, if you have ISOLATE or
> >> CMA here. There should at least be a !is_migrate_isolate_page() check
> >> somewhere
> >>
> >> 2. set_migratetype_isolate() takes the zone lock, so to avoid racing
> >> with isolation code, you have to hold the zone lock. Your code seems to
> >> do that, so at least you cannot race against isolation.
> >>
> >> 3. You could end up temporarily allocating something in the
> >> ZONE_MOVABLE. The pages you allocate are, however, not movable. There
> >> would have to be a way to make alloc_contig_range()/offlining code
> >> properly wait until the pages have been processed. Not sure about the
> >> real implications, though - too many details in the code (I wonder if
> >> Alex' series has a way of dealing with that)
> >>
> >> When you restore the migratetype, you could suddenly overwrite e.g.,
> >> ISOLATE, which feels wrong.
> >
> >
> > I was triggering an occasional CPU stall bug earlier, with saving and restoring
> > the migratetype I was able to fix it.
> > But I will further look into this to figure out if it is really required.
> >
>
> You should especially look into handling isolated/cma pages. Maybe that
> was the original issue. Alex seems to have added that in his latest
> series (skipping isolated/cma pageblocks completely) as well.

So as far as skipping isolated pageblocks, I get the reason for
skipping isolated, but why would we need to skip CMA? I had made the
change I did based on comments you had made earlier. But while working
on some of the changes to address isolation better and looking over
several spots in the code it seems like CMA is already being used as
an allocation fallback for MIGRATE_MOVABLE. If that is the case
wouldn't it make sense to allow pulling pages and reporting them while
they are in the free_list?

  parent reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 13:12 [RFC][PATCH v12 0/2] mm: Support for page reporting Nitesh Narayan Lal
2019-08-12 13:12 ` [RFC][Patch v12 1/2] mm: page_reporting: core infrastructure Nitesh Narayan Lal
2019-08-12 18:47   ` Alexander Duyck
2019-08-12 20:04     ` Nitesh Narayan Lal
2019-08-20 14:11       ` Nitesh Narayan Lal
2019-08-12 20:05     ` David Hildenbrand
2019-08-13 10:30       ` Nitesh Narayan Lal
2019-08-13 10:34         ` David Hildenbrand
2019-08-13 10:42           ` Nitesh Narayan Lal
2019-08-13 10:44             ` David Hildenbrand
2019-08-13 23:14           ` Alexander Duyck [this message]
2019-08-14  7:07             ` David Hildenbrand
2019-08-14 12:49               ` [virtio-dev] " Nitesh Narayan Lal
2019-08-14 15:49     ` Nitesh Narayan Lal
2019-08-14 16:11       ` Alexander Duyck
2019-08-15 13:15         ` Nitesh Narayan Lal
2019-08-15 19:22           ` Nitesh Narayan Lal
2019-08-15 23:00             ` Alexander Duyck
2019-08-16 18:35               ` Nitesh Narayan Lal
2019-08-30 15:15     ` Nitesh Narayan Lal
2019-08-30 15:31       ` Alexander Duyck
2019-08-30 16:05         ` Nitesh Narayan Lal
2019-09-04  8:40           ` [virtio-dev] " David Hildenbrand
2019-10-10 20:36   ` Alexander Duyck
2019-10-11 11:02     ` Nitesh Narayan Lal
2019-08-12 13:12 ` [RFC][Patch v12 2/2] virtio-balloon: interface to support free page reporting Nitesh Narayan Lal
2019-08-14 10:29   ` Cornelia Huck
2019-08-14 11:47     ` Nitesh Narayan Lal
2019-08-14 13:42       ` Cornelia Huck
2019-08-14 14:01         ` Nitesh Narayan Lal
2019-08-12 13:13 ` [QEMU Patch 1/2] virtio-balloon: adding bit for page reporting support Nitesh Narayan Lal
2019-08-12 13:13   ` [QEMU Patch 2/2] virtio-balloon: support for handling page reporting Nitesh Narayan Lal
2019-08-12 15:18     ` Alexander Duyck
2019-08-12 15:26       ` Nitesh Narayan Lal
2019-09-11 12:30 ` [RFC][PATCH v12 0/2] mm: Support for " David Hildenbrand

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKgT0UfaaHrEaS2wsbdTuzCdCtSrM4Tx79w=dP8HPEnq+T7rtQ@mail.gmail.com' \
    --to=alexander.duyck@gmail.com \
    --cc=aarcange@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=dave.hansen@intel.com \
    --cc=david@redhat.com \
    --cc=dhildenb@redhat.com \
    --cc=dodgen@google.com \
    --cc=john.starks@microsoft.com \
    --cc=konrad.wilk@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=lcapitulino@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=mst@redhat.com \
    --cc=nitesh@redhat.com \
    --cc=pagupta@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=riel@surriel.com \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=wei.w.wang@intel.com \
    --cc=yang.zhang.wz@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git