linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
To: David Hildenbrand <david@redhat.com>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	kvm@vger.kernel.org, mst@redhat.com,
	linux-kernel@vger.kernel.org, willy@infradead.org,
	mhocko@kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org,
	mgorman@techsingularity.net, vbabka@suse.cz
Cc: yang.zhang.wz@gmail.com, nitesh@redhat.com,
	konrad.wilk@oracle.com, pagupta@redhat.com, riel@surriel.com,
	lcapitulino@redhat.com, dave.hansen@intel.com,
	wei.w.wang@intel.com, aarcange@redhat.com, pbonzini@redhat.com,
	dan.j.williams@intel.com, osalvador@suse.de
Subject: Re: [PATCH v15 3/7] mm: Add function __putback_isolated_page
Date: Mon, 16 Dec 2019 08:22:52 -0800	[thread overview]
Message-ID: <9eb9173278370dd604c4cefd30ed10be36600854.camel@linux.intel.com> (raw)
In-Reply-To: <cb49bbc7-b0c0-65cc-1d9d-a3aaef075650@redhat.com>

On Mon, 2019-12-16 at 12:36 +0100, David Hildenbrand wrote:
> [...]
> > +/**
> > + * __putback_isolated_page - Return a now-isolated page back where we got it
> > + * @page: Page that was isolated
> > + * @order: Order of the isolated page
> > + *
> > + * This function is meant to return a page pulled from the free lists via
> > + * __isolate_free_page back to the free lists they were pulled from.
> > + */
> > +void __putback_isolated_page(struct page *page, unsigned int order)
> > +{
> > +	struct zone *zone = page_zone(page);
> > +	unsigned long pfn;
> > +	unsigned int mt;
> > +
> > +	/* zone lock should be held when this function is called */
> > +	lockdep_assert_held(&zone->lock);
> > +
> > +	pfn = page_to_pfn(page);
> > +	mt = get_pfnblock_migratetype(page, pfn);
> 
> IMHO get_pageblock_migratetype() would be nicer - I guess the compiler
> will optimize out the double page_to_pfn().

The thing is I need the page_to_pfn call already in order to pass the
value to __free_one_page. With that being the case why not juse use
get_pfnblock_migratetype?

Also there are some scenarios where __page_to_pfn is not that simple a
call with us having to get the node ID so we can find the pgdat structure
to perform the calculation. I'm not sure the compiler would be ble to
figure out that the result is the same for both calls, so it is better to
make it explicit.

> > +
> > +	/* Return isolated page to tail of freelist. */
> > +	__free_one_page(page, pfn, zone, order, mt);
> > +}
> > +
> >  /*
> >   * Update NUMA hit/miss statistics
> >   *
> > diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> > index 04ee1663cdbe..d93d2be0070f 100644
> > --- a/mm/page_isolation.c
> > +++ b/mm/page_isolation.c
> > @@ -134,13 +134,11 @@ static void unset_migratetype_isolate(struct page *page, unsigned migratetype)
> >  		__mod_zone_freepage_state(zone, nr_pages, migratetype);
> >  	}
> >  	set_pageblock_migratetype(page, migratetype);
> > +	if (isolated_page)
> > +		__putback_isolated_page(page, order);
> >  	zone->nr_isolate_pageblock--;
> >  out:
> >  	spin_unlock_irqrestore(&zone->lock, flags);
> > -	if (isolated_page) {
> > -		post_alloc_hook(page, order, __GFP_MOVABLE);
> > -		__free_pages(page, order);
> > -	}
> 
> So If I get it right:
> 
> post_alloc_hook() does quite some stuff like
> - arch_alloc_page(page, order);
> - kernel_map_pages(page, 1 << order, 1)
> - kasan_alloc_pages()
> - kernel_poison_pages(1)
> - set_page_owner()
> 
> Which free_pages_prepare() will undo, like
> - reset_page_owner()
> - kernel_poison_pages(0)
> - arch_free_page()
> - kernel_map_pages()
> - kasan_free_nondeferred_pages()
> 
> Both would be skipped now - which sounds like the right thing to do IMHO
> (and smells like quite a performance improvement). I haven't verified if
> actually everything we skip in free_pages_prepare() is safe (I think it
> is, it seems to be mostly relevant for actually used/allocated pages).

That was kind of my thought on this. Basically the logic I was following
was that the code path will call move_freepages_block that bypasses all of
the above mentioned calls if the pages it is moving will not be merged. If
it is safe in that case my assumption is that it should be safe to just
call __putback_isolated_page in such a case as it also bypasses the block
above, but it supports merging the page with other pages that are already
on the freelist.


  reply	other threads:[~2019-12-16 16:22 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 16:22 [PATCH v15 0/7] mm / virtio: Provide support for free page reporting Alexander Duyck
2019-12-05 16:22 ` [PATCH v15 1/7] mm: Adjust shuffle code to allow for future coalescing Alexander Duyck
2019-12-05 16:22 ` [PATCH v15 2/7] mm: Use zone and order instead of free area in free_list manipulators Alexander Duyck
2019-12-05 16:22 ` [PATCH v15 3/7] mm: Add function __putback_isolated_page Alexander Duyck
2019-12-16 11:36   ` David Hildenbrand
2019-12-16 16:22     ` Alexander Duyck [this message]
2019-12-17 10:58       ` David Hildenbrand
2019-12-17 16:26         ` Alexander Duyck
2019-12-17 17:24           ` David Hildenbrand
2019-12-17 18:24             ` Alexander Duyck
2019-12-17 18:46               ` David Hildenbrand
2019-12-17 21:50                 ` Alexander Duyck
2019-12-05 16:22 ` [PATCH v15 4/7] mm: Introduce Reported pages Alexander Duyck
2019-12-16 10:17   ` Nitesh Narayan Lal
2019-12-16 16:28     ` Alexander Duyck
2019-12-17  8:55       ` Nitesh Narayan Lal
2019-12-17 16:31         ` Alexander Duyck
2019-12-18  7:31           ` Mel Gorman
2019-12-16 11:44   ` Nitesh Narayan Lal
2019-12-16 16:10     ` Alexander Duyck
2019-12-05 16:22 ` [PATCH v15 5/7] virtio-balloon: Pull page poisoning config out of free page hinting Alexander Duyck
2019-12-13  7:03   ` Michael S. Tsirkin
2019-12-05 16:22 ` [PATCH v15 6/7] virtio-balloon: Add support for providing free page reports to host Alexander Duyck
2019-12-13  7:08   ` Michael S. Tsirkin
2019-12-13 16:35     ` Alexander Duyck
2019-12-15  9:29       ` Michael S. Tsirkin
2019-12-13 10:15   ` David Hildenbrand
2019-12-13 16:37     ` Alexander Duyck
2019-12-05 16:23 ` [PATCH v15 7/7] mm: Add free page reporting documentation Alexander Duyck
2019-12-05 16:24 ` [PATCH v15 QEMU 1/3] virtio-ballon: Implement support for page poison tracking feature Alexander Duyck
2019-12-05 16:24 ` [PATCH v15 QEMU 2/3] virtio-balloon: Add bit to notify guest of unused page reporting Alexander Duyck
2019-12-05 16:24 ` [PATCH v15 QEMU 3/3] virtio-balloon: Provide a interface for " Alexander Duyck
2019-12-05 16:26 ` [PATCH v15 QEMU 4/3 RFC] memory: Add support for MADV_FREE as mechanism to lazy discard pages Alexander Duyck
2019-12-12 23:47 ` [PATCH v15 0/7] mm / virtio: Provide support for free page reporting Alexander Duyck
2019-12-13 10:00 ` David Hildenbrand
2019-12-13 11:08   ` Mel Gorman
2019-12-13 16:59     ` Alexander Duyck
2019-12-13 16:46   ` Alexander Duyck
2019-12-16 12:21     ` David Hildenbrand

Reply instructions:

You may reply publicly 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=9eb9173278370dd604c4cefd30ed10be36600854.camel@linux.intel.com \
    --to=alexander.h.duyck@linux.intel.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.duyck@gmail.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=david@redhat.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=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=mst@redhat.com \
    --cc=nitesh@redhat.com \
    --cc=osalvador@suse.de \
    --cc=pagupta@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=riel@surriel.com \
    --cc=vbabka@suse.cz \
    --cc=wei.w.wang@intel.com \
    --cc=willy@infradead.org \
    --cc=yang.zhang.wz@gmail.com \
    --subject='Re: [PATCH v15 3/7] mm: Add function __putback_isolated_page' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).