Re: [PATCH 2/2] memcg: Allow hard guarantee mode for low limit reclaim

Michal Hocko
2014-06-09T08:30:53Z

On Fri 06-06-14 11:29:14, Tejun Heo wrote:
> Hello, Michal.
>
> On Fri, Jun 06, 2014 at 04:46:50PM +0200, Michal Hocko wrote:
> > +choice
> > + prompt "Memory Resource Controller reclaim protection"
> > + depends on MEMCG
> > + help
>
> Why is this necessary?

It allows user/admin to set the default behavior.

> - This doesn't affect boot.
>
> - memcg requires runtime config *anyway*.
>
> - The config is inherited from the parent, so the default flipping
> isn't exactly difficult.
>
> Please drop the kconfig option.

How do you propose to tell the default then? Only at the runtime?
I really do not insist on the kconfig. I find it useful for a)
documentation purpose b) easy way to configure the default.

> > +static int mem_cgroup_write_reclaim_strategy(struct cgroup_subsys_state *css, struct cftype *cft,
> > + char *buffer)
> > +{
> > + struct mem_cgroup *memcg = mem_cgroup_from_css(css);
> > + int ret = 0;
> > +
> > + if (!strncmp(buffer, "low_limit_guarantee",
> > + sizeof("low_limit_guarantee"))) {
> > + memcg->hard_low_limit = true;
> > + } else if (!strncmp(buffer, "low_limit_best_effort",
> > + sizeof("low_limit_best_effort"))) {
> > + memcg->hard_low_limit = false;
> > + } else
> > + ret = -EINVAL;
> > +
> > + return ret;
> > +}
>
> So, ummm, this raises a big red flag for me. You're now implementing
> two behaviors in a mostly symmetric manner to soft/hard limits but
> choosing a completely different scheme in how they're configured
> without any rationale.

So what is your suggestion then? Using a global setting? Using a
separate knob? Something completely different?

> * Are you sure soft and hard guarantees aren't useful when used in
> combination? If so, why would that be the case?

This was a call from Google to have per-memcg setup AFAIR. Using
different reclaim protection on the global case vs. limit reclaim makes
a lot of sense to me. If this is a major obstacle then I am OK to drop
it and only have a global setting for now.

> * We have pressure monitoring interface which can be used for soft
> limit pressure monitoring.

Which one is that? I only know about oom_control triggered by the hard
limit pressure.

> How should breaching soft guarantee be
> factored into that? There doesn't seem to be any way of notifying
> that at the moment? Wouldn't we want that to be integrated into the
> same mechanism?

Yes, there is. We have a counter in memory.stat file which tells how
many times the limit has been breached.

> What scares me the most is that you don't even seem to have noticed
> the asymmetry and are proposing userland-facing interface without
> actually thinking things through. This is exactly how we've been
> getting into trouble.

This has been discussed up and down for the last _two_ years. I have
considered other options how to provide a very _useful_ feature users
are calling for. There is even general consensus among developers that
the feature is desirable and that the two modes (soft/hard) memory
protection are needed. Yet I would _really_ like to hear any
suggestion to get unstuck. It is far from useful to come and Nack this
_again_ without providing any alternative suggestions.

> For now, for everything.
>
> Nacked-by: Tejun Heo <tj@kernel.org>
>
> Thanks.
>
> --
> tejun

--
Michal Hocko
SUSE Labs