From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@suse.com, akpm@linux-foundation.org
Cc: linux-mm@kvack.org, xiyou.wangcong@gmail.com,
dave.hansen@intel.com, hannes@cmpxchg.org, mgorman@suse.de,
vbabka@suse.cz
Subject: Re: [PATCH] mm,page_alloc: Serialize warn_alloc() if schedulable.
Date: Sat, 8 Jul 2017 13:59:54 +0900 [thread overview]
Message-ID: <201707081359.JCD39510.OSVOHMFOFtLFQJ@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20170602071818.GA29840@dhcp22.suse.cz>
Michal Hocko wrote:
> On Thu 01-06-17 15:10:22, Andrew Morton wrote:
> > On Thu, 1 Jun 2017 15:28:08 +0200 Michal Hocko <mhocko@suse.com> wrote:
> >
> > > On Thu 01-06-17 22:11:13, Tetsuo Handa wrote:
> > > > Michal Hocko wrote:
> > > > > On Thu 01-06-17 20:43:47, Tetsuo Handa wrote:
> > > > > > Cong Wang has reported a lockup when running LTP memcg_stress test [1].
> > > > >
> > > > > This seems to be on an old and not pristine kernel. Does it happen also
> > > > > on the vanilla up-to-date kernel?
> > > >
> > > > 4.9 is not an old kernel! It might be close to the kernel version which
> > > > enterprise distributions would choose for their next long term supported
> > > > version.
> > > >
> > > > And please stop saying "can you reproduce your problem with latest
> > > > linux-next (or at least latest linux)?" Not everybody can use the vanilla
> > > > up-to-date kernel!
> > >
> > > The changelog mentioned that the source of stalls is not clear so this
> > > might be out-of-tree patches doing something wrong and dump_stack
> > > showing up just because it is called often. This wouldn't be the first
> > > time I have seen something like that. I am not really keen on adding
> > > heavy lifting for something that is not clearly debugged and based on
> > > hand waving and speculations.
> >
> > I'm thinking we should serialize warn_alloc anyway, to prevent the
> > output from concurrent calls getting all jumbled together?
>
> dump_stack already serializes concurrent calls.
>
> > I'm not sure I buy the "this isn't a mainline kernel" thing.
>
> The changelog doesn't really explain what is going on and only
> speculates that the excessive warn_alloc is the cause. The kernel is
> 4.9.23.el7.twitter.x86_64 which I suspect contains a lot of stuff on top
> of 4.9. So I would really _like_ to see whether this is reproducible
> with the upstream kernel. Especially when this is a LTP test.
>
> > warn_alloc() obviously isn't very robust, but we'd prefer that it be
> > robust to peculiar situations, wild-n-wacky kernel patches, etc. It's
> > a low-level thing and it should Just Work.
>
> Yes I would agree and if we have an evidence that warn_alloc is really
> the problem then I am all for fixing it. There is no such evidence yet.
> Note that dump_stack serialization might be unfair because there is no
> queuing. Is it possible that this is the problem? If yes we should
> rather fix that because that is arguably even more low-level routine than
> warn_alloc.
>
> That being said. I strongly believe that this patch is not properly
> justified, issue fully understood and as such a disagree with adding a
> new lock on those grounds.
>
> Until the above is resolved
> Nacked-by: Michal Hocko <mhocko@suse.com>
Apart from what happened to Cong Wang's case, I'm really bothered by jumbled
messages caused by concurrent warn_alloc() calls. My test case is using
linux-next without any out-of-tree patches. Thus, adding a new lock on those
grounds should be acceptable.
Quoting from http://lkml.kernel.org/r/20170705081956.GA14538@dhcp22.suse.cz :
Michal Hocko wrote:
> On Sat 01-07-17 20:43:56, Tetsuo Handa wrote:
> > You are rejecting serialization under OOM without giving a chance to test
> > side effects of serialization under OOM at linux-next.git. I call such attitude
> > "speculation" which you never accept.
>
> No I am rejecting abusing the lock for purpose it is not aimed for.
Then, why adding a new lock (not oom_lock but warn_alloc_lock) is not acceptable?
Since warn_alloc_lock is aimed for avoiding messages by warn_alloc() getting
jumbled, there should be no reason you reject this lock.
If you don't like locks, can you instead accept below one?
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 80e4adb..3ac382c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3900,9 +3900,14 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
/* Make sure we know about allocations which stall for too long */
if (time_after(jiffies, alloc_start + stall_timeout)) {
+ static bool wait;
+
+ while (cmpxchg(&wait, false, true))
+ schedule_timeout_uninterruptible(1);
warn_alloc(gfp_mask & ~__GFP_NOWARN, ac->nodemask,
"page allocation stalls for %ums, order:%u",
jiffies_to_msecs(jiffies-alloc_start), order);
+ wait = false;
stall_timeout += 10 * HZ;
}
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-07-08 5:00 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-01 11:43 [PATCH] mm,page_alloc: Serialize warn_alloc() if schedulable Tetsuo Handa
2017-06-01 11:59 ` Michal Hocko
2017-06-01 13:11 ` Tetsuo Handa
2017-06-01 13:28 ` Michal Hocko
2017-06-01 22:10 ` Andrew Morton
2017-06-02 7:18 ` Michal Hocko
2017-06-02 11:13 ` Tetsuo Handa
2017-06-02 12:15 ` Michal Hocko
2017-06-02 17:13 ` Tetsuo Handa
2017-06-02 21:57 ` Cong Wang
2017-06-04 8:58 ` Tetsuo Handa
2017-06-04 15:05 ` Michal Hocko
2017-06-04 21:43 ` Tetsuo Handa
2017-06-05 5:37 ` Michal Hocko
2017-06-05 18:15 ` Cong Wang
2017-06-06 9:17 ` Michal Hocko
2017-06-05 18:25 ` Cong Wang
2017-06-22 10:35 ` Tetsuo Handa
2017-06-22 22:53 ` Cong Wang
2017-06-02 16:59 ` Cong Wang
2017-06-02 19:59 ` Andrew Morton
2017-06-03 2:57 ` Tetsuo Handa
2017-06-03 7:32 ` Michal Hocko
2017-06-03 8:36 ` Tetsuo Handa
2017-06-05 7:10 ` Sergey Senozhatsky
2017-06-05 9:36 ` Sergey Senozhatsky
2017-06-05 15:02 ` Tetsuo Handa
2017-06-03 13:21 ` Tetsuo Handa
2017-07-08 4:59 ` Tetsuo Handa [this message]
2017-07-10 13:21 ` Michal Hocko
2017-07-10 13:54 ` Tetsuo Handa
2017-07-10 14:14 ` Michal Hocko
2017-07-11 13:10 ` Tetsuo Handa
2017-07-11 13:49 ` Michal Hocko
2017-07-11 14:58 ` Petr Mladek
2017-07-11 22:06 ` Tetsuo Handa
2017-07-12 8:54 ` Michal Hocko
2017-07-12 12:23 ` Tetsuo Handa
2017-07-12 12:41 ` Michal Hocko
2017-07-14 12:30 ` Tetsuo Handa
2017-07-14 12:48 ` Michal Hocko
2017-08-09 6:14 ` Tetsuo Handa
2017-08-09 13:01 ` Tetsuo Handa
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=201707081359.JCD39510.OSVOHMFOFtLFQJ@I-love.SAKURA.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=akpm@linux-foundation.org \
--cc=dave.hansen@intel.com \
--cc=hannes@cmpxchg.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mhocko@suse.com \
--cc=vbabka@suse.cz \
--cc=xiyou.wangcong@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).