[Devel] [PATCH 2/7] tcp: Charge socket buffers into cg memory (v2)

[Vladimir Davydov]

On Thu, Jun 04, 2015 at 12:45:24PM +0300, Pavel Emelyanov wrote:
> On 06/04/2015 11:56 AM, Vladimir Davydov wrote:
> > On Wed, Jun 03, 2015 at 04:34:21PM +0300, Pavel Emelyanov wrote:
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 9dda309..d38868c 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -498,7 +498,6 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
> >>  
> >>  /* Writing them here to avoid exposing memcg's inner layout */
> >>  #if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
> >> -
> >>  void sock_update_memcg(struct sock *sk)
> >>  {
> >>  	if (mem_cgroup_sockets_enabled) {
> >> @@ -3039,11 +3038,33 @@ int memcg_charge_kmem(struct mem_cgroup *memcg, gfp_t gfp, u64 size)
> >>  	return ret;
> >>  }
> >>  
> >> -void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
> > 
> > I've changed this code recently, so this patch does not apply. Please
> > update.
> 
> :( If this is just context change, then Kostja can just manually fix it. Please.
> 
> >> +void memcg_charge_kmem_nofail(struct mem_cgroup *memcg, u64 size)
> >>  {
> >> +	struct res_counter *fail_res;
> >> +
> >> +	/*
> >> +	 * FIXME -- strictly speaking, this value should _also_
> >> +	 * be charged into kmem counter. But since res_counter_charge
> >> +	 * is sub-optimal (takes locks) AND we do not care much
> >> +	 * about kmem limits (at least for now) we can just directly
> >> +	 * charge into mem counter.
> >> +	 */
> > 
> > Please charge kmem too. As I've already told you it should not make any
> > difference in terms of performance, because we already have a bottleneck
> > of the same bandwidth.
> > 
> > Anyway, if we see any performance degradation, I will convert
> > mem_cgroup->kmem to a percpu counter.
> 
> No, let's do it vice-versa -- first you fix the locking, then I update this code.

I don't understand why, because you provide no arguments and keep
ignoring my reasoning why I think charging kmem along with res is OK,
which is one paragraph above.