All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@stericsson.com>
To: Mel Gorman <mel@csn.ul.ie>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linus WALLEIJ <linus.walleij@stericsson.com>,
	Andrea GALLO <andrea.gallo@stericsson.com>,
	Vincent GUITTOT <vincent.guittot@stericsson.com>,
	Philippe LANGLAIS <philippe.langlais@stericsson.com>,
	Loic PALLARDY <loic.pallardy@stericsson.com>
Subject: Re: [RFCv1 3/6] PASR: mm: Integrate PASR in Buddy allocator
Date: Tue, 31 Jan 2012 13:15:55 +0100	[thread overview]
Message-ID: <4F27DB7B.4010103@stericsson.com> (raw)
In-Reply-To: <4F26CAD1.2000209@stericsson.com>

Hello Mel,
On 01/30/2012 05:52 PM, Maxime Coquelin wrote:
>
> On 01/30/2012 04:22 PM, Mel Gorman wrote:
>
>> You may be able to use the existing arch_alloc_page() hook and
>> call PASR on architectures that support it if and only if PASR is
>> present and enabled by the administrator but even this is likely to be
>> unpopular as it'll have a measurable performance impact on platforms
>> with PASR (not to mention the PASR lock will be even heavier as it'll
>> now be also used for per-cpu page allocations). To get the hook you
>> want, you'd need to show significant benefit before they were happy with
>> the hook.
> Your proposal sounds good.
> AFAIK, per-cpu allocation maximum size is 32KB. Please correct me if 
> I'm wrong.
> Since pasr_kget/kput() calls the PASR framework only on MAX_ORDER 
> allocations, we wouldn't add any locking risks nor contention compared 
> to current patch.
> I will update the patch set using  arch_alloc/free_page().
>
I just had a deeper look at when arch_alloc_page() is called. I think it 
does not fit with PASR framework needs.
pasr_kget() calls pasr_get() only for max order pages (same for 
pasr_kput()) to avoid overhead.

In current patch set, pasr_kget() is called when pages are removed from 
the free lists, and pasr_kput() when pages are inserted in the free lists.
So, pasr_get() is called in case of :
     - allocation of a max order page
     - split of a max order page into lower order pages to fulfill 
allocation of pages smaller than max order
And pasr_put() is called in case of:
     - release of a max order page
     - coalescence of two "max order -1" pages when smaller pages are 
released

If we call the PASR framework in arch_alloc_page(), we have two 
possibilities:
     1) using pasr_kget(): the PASR framework will only be notified of 
max order allocations, so the coalesce/split of free pages case will not 
be taken into account.
     2) using pasr_get(): the PASR framework will be called for every 
orders of page allocation/release. The induced overhead is not acceptable.

To avoid calling pasr_kget/kput() directly in page_alloc.c, do you think 
adding some arch specific hooks when a page is inserted or removed from 
the free lists could be acceptable?
Something like arch_insert_freepage(struct page *page, int order) and 
arch_remove_freepage(struct page *page, int order).

Regards,
Maxime

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Coquelin <maxime.coquelin@stericsson.com>
To: Mel Gorman <mel@csn.ul.ie>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linus WALLEIJ <linus.walleij@stericsson.com>,
	Andrea GALLO <andrea.gallo@stericsson.com>,
	Vincent GUITTOT <vincent.guittot@stericsson.com>,
	Philippe LANGLAIS <philippe.langlais@stericsson.com>,
	Loic PALLARDY <loic.pallardy@stericsson.com>
Subject: Re: [RFCv1 3/6] PASR: mm: Integrate PASR in Buddy allocator
Date: Tue, 31 Jan 2012 13:15:55 +0100	[thread overview]
Message-ID: <4F27DB7B.4010103@stericsson.com> (raw)
In-Reply-To: <4F26CAD1.2000209@stericsson.com>

Hello Mel,
On 01/30/2012 05:52 PM, Maxime Coquelin wrote:
>
> On 01/30/2012 04:22 PM, Mel Gorman wrote:
>
>> You may be able to use the existing arch_alloc_page() hook and
>> call PASR on architectures that support it if and only if PASR is
>> present and enabled by the administrator but even this is likely to be
>> unpopular as it'll have a measurable performance impact on platforms
>> with PASR (not to mention the PASR lock will be even heavier as it'll
>> now be also used for per-cpu page allocations). To get the hook you
>> want, you'd need to show significant benefit before they were happy with
>> the hook.
> Your proposal sounds good.
> AFAIK, per-cpu allocation maximum size is 32KB. Please correct me if 
> I'm wrong.
> Since pasr_kget/kput() calls the PASR framework only on MAX_ORDER 
> allocations, we wouldn't add any locking risks nor contention compared 
> to current patch.
> I will update the patch set using  arch_alloc/free_page().
>
I just had a deeper look at when arch_alloc_page() is called. I think it 
does not fit with PASR framework needs.
pasr_kget() calls pasr_get() only for max order pages (same for 
pasr_kput()) to avoid overhead.

In current patch set, pasr_kget() is called when pages are removed from 
the free lists, and pasr_kput() when pages are inserted in the free lists.
So, pasr_get() is called in case of :
     - allocation of a max order page
     - split of a max order page into lower order pages to fulfill 
allocation of pages smaller than max order
And pasr_put() is called in case of:
     - release of a max order page
     - coalescence of two "max order -1" pages when smaller pages are 
released

If we call the PASR framework in arch_alloc_page(), we have two 
possibilities:
     1) using pasr_kget(): the PASR framework will only be notified of 
max order allocations, so the coalesce/split of free pages case will not 
be taken into account.
     2) using pasr_get(): the PASR framework will be called for every 
orders of page allocation/release. The induced overhead is not acceptable.

To avoid calling pasr_kget/kput() directly in page_alloc.c, do you think 
adding some arch specific hooks when a page is inserted or removed from 
the free lists could be acceptable?
Something like arch_insert_freepage(struct page *page, int order) and 
arch_remove_freepage(struct page *page, int order).

Regards,
Maxime

  reply	other threads:[~2012-01-31 12:16 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-30 13:33 [RFCv1 0/6] PASR: Partial Array Self-Refresh Framework Maxime Coquelin
2012-01-30 13:33 ` Maxime Coquelin
2012-01-30 13:33 ` [RFCv1 1/6] PASR: Initialize DDR layout Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-01-30 13:33 ` [RFCv1 2/6] PASR: Add core Framework Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-01-30 13:33 ` [RFCv1 3/6] PASR: mm: Integrate PASR in Buddy allocator Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-01-30 15:22   ` Mel Gorman
2012-01-30 15:22     ` Mel Gorman
2012-01-30 16:52     ` Maxime Coquelin
2012-01-30 16:52       ` Maxime Coquelin
2012-01-31 12:15       ` Maxime Coquelin [this message]
2012-01-31 12:15         ` Maxime Coquelin
2012-01-31 14:01         ` Mel Gorman
2012-01-31 14:01           ` Mel Gorman
2012-01-31 18:55           ` Maxime Coquelin
2012-01-31 18:55             ` Maxime Coquelin
2012-02-03 11:44             ` Mel Gorman
2012-02-03 11:44               ` Mel Gorman
2012-01-31 12:46       ` Pekka Enberg
2012-01-31 12:46         ` Pekka Enberg
2012-01-30 13:33 ` [RFCv1 4/6] PASR: Call PASR initialization Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-01-30 13:33 ` [RFCv1 5/6] PASR: Add Documentation Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-02-02  3:51   ` Randy Dunlap
2012-02-02  3:51     ` Randy Dunlap
2012-02-02 17:12     ` Maxime Coquelin
2012-02-02 17:12       ` Maxime Coquelin
2012-01-30 13:33 ` [RFCv1 6/6] PASR: Ux500: Add PASR support Maxime Coquelin
2012-01-30 13:33   ` Maxime Coquelin
2012-01-30 13:53 ` [RFCv1 0/6] PASR: Partial Array Self-Refresh Framework Ingo Molnar
2012-01-30 13:53   ` Ingo Molnar
2012-01-30 14:19   ` Maxime Coquelin
2012-01-30 14:19     ` Maxime Coquelin
2012-01-31 12:39     ` Ingo Molnar
2012-01-31 12:39       ` Ingo Molnar
2012-01-31 14:48       ` Maxime Coquelin
2012-01-31 14:48         ` Maxime Coquelin

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=4F27DB7B.4010103@stericsson.com \
    --to=maxime.coquelin@stericsson.com \
    --cc=andrea.gallo@stericsson.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=loic.pallardy@stericsson.com \
    --cc=mel@csn.ul.ie \
    --cc=philippe.langlais@stericsson.com \
    --cc=vincent.guittot@stericsson.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.