All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Michal Hocko <mhocko@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH] mm:memcg: add __GFP_NOWARN in __memcg_schedule_kmem_cache_create
Date: Wed, 18 Apr 2018 22:23:28 +0900	[thread overview]
Message-ID: <20180418132328.GB210164@rodete-desktop-imager.corp.google.com> (raw)
In-Reply-To: <20180418075437.GP17484@dhcp22.suse.cz>

On Wed, Apr 18, 2018 at 09:54:37AM +0200, Michal Hocko wrote:
> On Wed 18-04-18 16:41:17, Minchan Kim wrote:
> > On Wed, Apr 18, 2018 at 09:20:02AM +0200, Michal Hocko wrote:
> > > On Wed 18-04-18 11:29:12, Minchan Kim wrote:
> [...]
> > > > Let's not make user scared.
> > > 
> > > This is not a proper explanation. So what exactly happens when this
> > > allocation fails? I would suggest something like the following
> > > "
> > > __memcg_schedule_kmem_cache_create tries to create a shadow slab cache
> > > and the worker allocation failure is not really critical because we will
> > > retry on the next kmem charge. We might miss some charges but that
> > > shouldn't be critical. The excessive allocation failure report is not
> > > very much helpful. Replace it with a rate limited single line output so
> > > that we know that there is a lot of these failures and that we need to
> > > do something about it in future.
> > > "
> > > 
> > > With the last part to be implemented of course.
> > 
> > If you want to see warning and catch on it in future, I don't see any reason
> > to change it. Because I didn't see any excessive warning output that it could
> > make system slow unless we did ratelimiting.
> 
> Yeah, but a single line would be as much informative and less scary to
> users.
> 
> > It was a just report from non-MM guys who have a concern that somethings
> > might go wrong on the system. I just wanted them relax since it's not
> > critical.
> 
> I do agree with __GFP_NOWARN but I think a single line warning is due
> and helpful for further debugging.

Okay, no problem. However, I don't feel we need ratelimit at this moment.
We can do when we got real report. Let's add just one line warning.
However, I have no talent to write a poem to express with one line.
Could you help me?

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 671d07e73a3b..e26f85cac63f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2201,8 +2201,11 @@ static void __memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg,
        struct memcg_kmem_cache_create_work *cw;

        cw = kmalloc(sizeof(*cw), GFP_NOWAIT | __GFP_NOWARN);
-       if (!cw)
+       if (!cw) {
+               pr_warn("Fail to create shadow slab cache for memcg but it's not critical.\n");
+               pr_warn("If you see lots of this message, send an email to linux-mm@kvack.org\n");
                return;
+       }

        css_get(&memcg->css);

  reply	other threads:[~2018-04-18 13:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-18  2:29 [PATCH] mm:memcg: add __GFP_NOWARN in __memcg_schedule_kmem_cache_create Minchan Kim
2018-04-18  2:56 ` David Rientjes
2018-04-18  3:08 ` Matthew Wilcox
2018-04-18  4:16   ` David Rientjes
2018-04-18  7:09   ` Michal Hocko
2018-04-18  7:20 ` Michal Hocko
2018-04-18  7:41   ` Minchan Kim
2018-04-18  7:54     ` Michal Hocko
2018-04-18 13:23       ` Minchan Kim [this message]
2018-04-18 13:27         ` Michal Hocko
2018-04-18 18:58           ` David Rientjes
2018-04-19  6:40             ` Michal Hocko
2018-04-20  5:42               ` Minchan Kim
2018-04-20  8:09                 ` Michal Hocko
2018-04-18 13:31 ` Matthew Wilcox
2018-04-18 13:39   ` Michal Hocko
2018-04-18 19:05 ` Johannes Weiner

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=20180418132328.GB210164@rodete-desktop-imager.corp.google.com \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@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 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.