All of lore.kernel.org
 help / color / mirror / Atom feed
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
To: minchan.kim@gmail.com
Cc: abarry@cray.com, akpm@linux-foundation.org, linux-mm@kvack.org,
	mgorman@suse.de, riel@redhat.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, fengguang.wu@intel.com
Subject: Re: Unending loop in __alloc_pages_slowpath following OOM-kill; rfc: patch.
Date: Tue, 24 May 2011 17:41:44 +0900	[thread overview]
Message-ID: <4DDB6F48.1010809@jp.fujitsu.com> (raw)
In-Reply-To: <BANLkTinkcu5j1H8tHNT4aTmOL-GXfSwPQw@mail.gmail.com>

>> I'm sorry I missed this thread long time.
> 
> No problem. It would be better than not review.

thx.


>> In this case, I think we should call drain_all_pages(). then following
>> patch is better.
> 
> Strictly speaking, this problem isn't related to drain_all_pages.
> This problem caused by lru empty but I admit it could work well if
> your patch applied.
> So yours could help, too.
> 
>> However I also think your patch is valuable. because while the task is
>> sleeping in wait_iff_congested(), an another task may free some pages.
>> thus, rebalance path should try to get free pages. iow, you makes sense.
> 
> Yes.
> Off-topic.
> I would like to move cond_resched below get_page_from_freelist in
> __alloc_pages_direct_reclaim. Otherwise, it is likely we can be stolen
> pages to other processes.
> One more benefit is that if it's apparently OOM path(ie,
> did_some_progress = 0), we can reduce OOM kill latency due to remove
> unnecessary cond_resched.

I agree. Can you please mind to send a patch?


>> So, I'd like to propose to merge both your and my patch.
> 
> Recently, there was discussion on drain_all_pages with Wu.
> He saw much overhead in 8-core system, AFAIR.
> I Cced Wu.
> 
> How about checking per-cpu before calling drain_all_pages() than
> unconditional calling?
> if (per_cpu_ptr(zone->pageset, smp_processor_id())
>     drain_all_pages();
> 
> Of course, It can miss other CPU free pages. But above routine assume
> local cpu direct reclaim is successful but it failed by per-cpu. So I
> think it works.

Can you please tell me previous discussion url or mail subject?
I mean, if it is costly and performance degression risk, we don't have to
take my idea.

Thanks.


> 
> Thanks for good suggestion and Reviewed-by, KOSAKI.



WARNING: multiple messages have this Message-ID (diff)
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
To: minchan.kim@gmail.com
Cc: abarry@cray.com, akpm@linux-foundation.org, linux-mm@kvack.org,
	mgorman@suse.de, riel@redhat.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, fengguang.wu@intel.com
Subject: Re: Unending loop in __alloc_pages_slowpath following OOM-kill; rfc: patch.
Date: Tue, 24 May 2011 17:41:44 +0900	[thread overview]
Message-ID: <4DDB6F48.1010809@jp.fujitsu.com> (raw)
In-Reply-To: <BANLkTinkcu5j1H8tHNT4aTmOL-GXfSwPQw@mail.gmail.com>

>> I'm sorry I missed this thread long time.
> 
> No problem. It would be better than not review.

thx.


>> In this case, I think we should call drain_all_pages(). then following
>> patch is better.
> 
> Strictly speaking, this problem isn't related to drain_all_pages.
> This problem caused by lru empty but I admit it could work well if
> your patch applied.
> So yours could help, too.
> 
>> However I also think your patch is valuable. because while the task is
>> sleeping in wait_iff_congested(), an another task may free some pages.
>> thus, rebalance path should try to get free pages. iow, you makes sense.
> 
> Yes.
> Off-topic.
> I would like to move cond_resched below get_page_from_freelist in
> __alloc_pages_direct_reclaim. Otherwise, it is likely we can be stolen
> pages to other processes.
> One more benefit is that if it's apparently OOM path(ie,
> did_some_progress = 0), we can reduce OOM kill latency due to remove
> unnecessary cond_resched.

I agree. Can you please mind to send a patch?


>> So, I'd like to propose to merge both your and my patch.
> 
> Recently, there was discussion on drain_all_pages with Wu.
> He saw much overhead in 8-core system, AFAIR.
> I Cced Wu.
> 
> How about checking per-cpu before calling drain_all_pages() than
> unconditional calling?
> if (per_cpu_ptr(zone->pageset, smp_processor_id())
>     drain_all_pages();
> 
> Of course, It can miss other CPU free pages. But above routine assume
> local cpu direct reclaim is successful but it failed by per-cpu. So I
> think it works.

Can you please tell me previous discussion url or mail subject?
I mean, if it is costly and performance degression risk, we don't have to
take my idea.

Thanks.


> 
> Thanks for good suggestion and Reviewed-by, KOSAKI.


--
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>

  reply	other threads:[~2011-05-24  8:41 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-13 21:31 Unending loop in __alloc_pages_slowpath following OOM-kill; rfc: patch Andrew Barry
2011-05-17 10:34 ` Minchan Kim
2011-05-17 11:34   ` Mel Gorman
2011-05-17 15:49   ` Andrew Barry
2011-05-18 22:29     ` Minchan Kim
2011-05-20 16:49       ` Minchan Kim
2011-05-20 16:49         ` Minchan Kim
2011-05-20 17:16         ` Rik van Riel
2011-05-20 17:16           ` Rik van Riel
2011-05-20 17:23         ` Mel Gorman
2011-05-20 17:23           ` Mel Gorman
2011-05-24  4:54         ` KOSAKI Motohiro
2011-05-24  4:54           ` KOSAKI Motohiro
2011-05-24  5:45           ` KOSAKI Motohiro
2011-05-24  5:45             ` KOSAKI Motohiro
2011-05-24  8:30           ` Mel Gorman
2011-05-24  8:30             ` Mel Gorman
2011-05-24  8:36             ` KOSAKI Motohiro
2011-05-24  8:36               ` KOSAKI Motohiro
2011-05-24  8:49               ` Mel Gorman
2011-05-24  8:49                 ` Mel Gorman
2011-05-24  9:05                 ` KOSAKI Motohiro
2011-05-24  9:05                   ` KOSAKI Motohiro
2011-05-24  9:16                   ` Mel Gorman
2011-05-24  9:16                     ` Mel Gorman
2011-05-24  9:40                     ` KOSAKI Motohiro
2011-05-24  9:40                       ` KOSAKI Motohiro
2011-05-24 10:57                       ` Mel Gorman
2011-05-24 10:57                         ` Mel Gorman
2011-05-24 23:53                         ` KOSAKI Motohiro
2011-05-24 23:53                           ` KOSAKI Motohiro
2011-05-24  8:34           ` Minchan Kim
2011-05-24  8:34             ` Minchan Kim
2011-05-24  8:41             ` KOSAKI Motohiro [this message]
2011-05-24  8:41               ` KOSAKI Motohiro
2011-05-24  8:57               ` Minchan Kim
2011-05-24  8:57                 ` Minchan Kim
2011-05-24  9:36                 ` KOSAKI Motohiro
2011-05-24  9:36                   ` KOSAKI Motohiro

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=4DDB6F48.1010809@jp.fujitsu.com \
    --to=kosaki.motohiro@jp.fujitsu.com \
    --cc=abarry@cray.com \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan.kim@gmail.com \
    --cc=riel@redhat.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.