From: Michal Hocko <mhocko@suse.cz>
To: Vladimir Davydov <vdavydov@parallels.com>
Cc: akpm@linux-foundation.org, rientjes@google.com,
penberg@kernel.org, cl@linux.com, glommer@gmail.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
devel@openvz.org
Subject: Re: [PATCH v2 3/7] memcg, slab: separate memcg vs root cache creation paths
Date: Thu, 6 Feb 2014 19:17:35 +0100 [thread overview]
Message-ID: <20140206181735.GA2137@dhcp22.suse.cz> (raw)
In-Reply-To: <52F3C293.3060400@parallels.com>
On Thu 06-02-14 21:12:51, Vladimir Davydov wrote:
> On 02/06/2014 08:41 PM, Michal Hocko wrote:
[...]
> >> +int kmem_cache_create_memcg(struct mem_cgroup *memcg, struct kmem_cache *cachep)
> >> {
> >> - return kmem_cache_create_memcg(NULL, name, size, align, flags, ctor, NULL);
> >> + struct kmem_cache *s;
> >> + int err;
> >> +
> >> + get_online_cpus();
> >> + mutex_lock(&slab_mutex);
> >> +
> >> + /*
> >> + * Since per-memcg caches are created asynchronously on first
> >> + * allocation (see memcg_kmem_get_cache()), several threads can try to
> >> + * create the same cache, but only one of them may succeed.
> >> + */
> >> + err = -EEXIST;
> > Does it make any sense to report the error here? If we are racing then at
> > least on part wins and the work is done.
>
> Yeah, you're perfectly right. It's better to return 0 here.
Why not void?
> > We should probably warn about errors which prevent from accounting but
> > I do not think there is much more we can do so returning an error code
> > from this function seems pointless. memcg_create_cache_work_func ignores
> > the return value anyway.
>
> I do not think warnings are appropriate here, because it is not actually
> an error if we are short on memory and can't do proper memcg accounting
> due to this. Perhaps, we'd better add fail counters for memcg cache
> creations and/or accounting to the root cache instead of memcg's one.
> That would be useful for debugging. I'm not sure though.
warn on once per memcg would be probably sufficient but it would still
be great if an admin could see that a memcg is not accounted although it
is supposed to be. Scanning all the memcgs might be really impractical.
We do not fail allocations needed for those object in the real life now
but we shouldn't rely on that.
--
Michal Hocko
SUSE Labs
--
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:[~2014-02-06 23:45 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-03 15:54 [PATCH v2 0/7] memcg-vs-slab related fixes, improvements, cleanups Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 1/7] memcg, slab: never try to merge memcg caches Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 2/7] memcg, slab: cleanup memcg cache name creation Vladimir Davydov
2014-02-03 22:08 ` Andrew Morton
2014-02-04 6:27 ` Vladimir Davydov
2014-02-04 7:39 ` [PATCH] memcg, slab: cleanup memcg cache creation Vladimir Davydov
2014-02-04 15:33 ` Michal Hocko
2014-02-04 16:09 ` Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 3/7] memcg, slab: separate memcg vs root cache creation paths Vladimir Davydov
2014-02-04 16:03 ` Michal Hocko
2014-02-04 19:19 ` Vladimir Davydov
2014-02-06 16:41 ` Michal Hocko
2014-02-06 17:12 ` Vladimir Davydov
2014-02-06 18:17 ` Michal Hocko [this message]
2014-02-06 18:43 ` Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 4/7] memcg, slab: unregister cache from memcg before starting to destroy it Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 5/7] memcg, slab: do not destroy children caches if parent has aliases Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 6/7] slub: adjust memcg caches when creating cache alias Vladimir Davydov
2014-02-03 15:54 ` [PATCH v2 7/7] slub: rework sysfs layout for memcg caches Vladimir Davydov
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=20140206181735.GA2137@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=devel@openvz.org \
--cc=glommer@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=vdavydov@parallels.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