From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6CAB5C4724C for ; Thu, 7 May 2020 21:03:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2A44020661 for ; Thu, 7 May 2020 21:03:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg-org.20150623.gappssmtp.com header.i=@cmpxchg-org.20150623.gappssmtp.com header.b="I8fAtO4C" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2A44020661 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A79CD900004; Thu, 7 May 2020 17:03:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A2A28900002; Thu, 7 May 2020 17:03:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8F288900004; Thu, 7 May 2020 17:03:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0207.hostedemail.com [216.40.44.207]) by kanga.kvack.org (Postfix) with ESMTP id 78FE2900002 for ; Thu, 7 May 2020 17:03:32 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 2A8D516479 for ; Thu, 7 May 2020 21:03:32 +0000 (UTC) X-FDA: 76791149064.01.drop95_3bdd555c2eb24 X-HE-Tag: drop95_3bdd555c2eb24 X-Filterd-Recvd-Size: 6033 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by imf18.hostedemail.com (Postfix) with ESMTP for ; Thu, 7 May 2020 21:03:31 +0000 (UTC) Received: by mail-qk1-f193.google.com with SMTP id s9so1312027qkm.6 for ; Thu, 07 May 2020 14:03:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=qcKYLVt6/NNJuQkZbbwTmTKpZql0R6iO8Dr7nx85OkM=; b=I8fAtO4ClEbBWt+UUO3Dwt+srJXW10D9hX2anKmgKYvAo4vvj4pxdGmu17H/mzJ6mn FAg81y3+hc6lkj085UoPx4ih3+p+IMmEZFpbiZKE1l7Y6nNExdpBkeczCez7DVX+EO7M gUZTcSRWuZkQMZXUkCBgHteJyr51toiodbFaZL4gZqez/5V8kp9Zfj93sgH2L2KRwZfH +aPXM9V7cmMy8kPifCYLzy53IybZC/67MsJO3X3nuLGocXZpO4C7xL9Z2fpa65sXg/Al 6Xyvi8JWx9Ml5Myj9DY/ItaJRUrFepMxsmAh+bDewEx2JXKnD7Jj0aAAWsWLCJO2euk7 Z4Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=qcKYLVt6/NNJuQkZbbwTmTKpZql0R6iO8Dr7nx85OkM=; b=SSth2VcViN7U95UG8w2uEn/zqfb1WiPIv1oLjZkrikRcheg3oiC6/JynmAyKEM8Kqe hMoH9NCKfapkwObzJkZOIkcuxyHgqHhcNbxYqw6Nr2Us5XAhzpbDLSI0Ji/ksyclrwQa iJ+nQgIJX0p56ouPuyl9jXtgeAahCsQNfzEzNJzFrGEjY7NNzzLBCUe4V/SikDzLOhZD UHlAneBOqylz009NWt4L+IMml6byI/CqFehcvWNGwFgEHBhLq+Sy3fKegqXzczYX5jAQ GOizQ9aICwR0QPWr7f4wFEd0UM/kdFTDqqxKWJb0LPHJslSO7Ox5IxrXHEziAdVOal1k +hFg== X-Gm-Message-State: AGi0PuaRHsvCcet7P2jaXYhyUtLBkwNObR3F2MJPEBYTZBZWAUHhMvPB nsfGtYfN5Cs+JJYOd8lmjLxLty3lCKE= X-Google-Smtp-Source: APiQypItvXGM4KItpj88RhB6zhQ84OCvNwUjPostRndJDyIpGjl/afmtOuDKI4u1nGRdx60YieiyGA== X-Received: by 2002:a05:620a:12dc:: with SMTP id e28mr17201367qkl.38.1588885410496; Thu, 07 May 2020 14:03:30 -0700 (PDT) Received: from localhost (70.44.39.90.res-cmts.bus.ptd.net. [70.44.39.90]) by smtp.gmail.com with ESMTPSA id 14sm5128001qkd.70.2020.05.07.14.03.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 May 2020 14:03:29 -0700 (PDT) Date: Thu, 7 May 2020 17:03:14 -0400 From: Johannes Weiner To: Roman Gushchin Cc: Andrew Morton , Michal Hocko , linux-mm@kvack.org, kernel-team@fb.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 06/19] mm: memcg/slab: obj_cgroup API Message-ID: <20200507210314.GD161043@cmpxchg.org> References: <20200422204708.2176080-1-guro@fb.com> <20200422204708.2176080-7-guro@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200422204708.2176080-7-guro@fb.com> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 22, 2020 at 01:46:55PM -0700, Roman Gushchin wrote: > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -257,6 +257,78 @@ struct cgroup_subsys_state *vmpressure_to_css(struct vmpressure *vmpr) > } > > #ifdef CONFIG_MEMCG_KMEM > +extern spinlock_t css_set_lock; > + > +static void obj_cgroup_release(struct percpu_ref *ref) > +{ > + struct obj_cgroup *objcg = container_of(ref, struct obj_cgroup, refcnt); > + struct mem_cgroup *memcg; > + unsigned int nr_bytes; > + unsigned int nr_pages; > + unsigned long flags; > + > + nr_bytes = atomic_read(&objcg->nr_charged_bytes); > + WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1)); > + nr_pages = nr_bytes >> PAGE_SHIFT; What guarantees that we don't have a partial page in there at this point? I guess any outstanding allocations would pin the objcg, so when it's released all objects have been freed. But if that's true, how can we have full pages remaining in there now? > @@ -2723,6 +2820,30 @@ struct mem_cgroup *mem_cgroup_from_obj(void *p) > return page->mem_cgroup; > } > > +__always_inline struct obj_cgroup *get_obj_cgroup_from_current(void) > +{ > + struct obj_cgroup *objcg = NULL; > + struct mem_cgroup *memcg; > + > + if (unlikely(!current->mm)) > + return NULL; > + > + rcu_read_lock(); > + if (unlikely(current->active_memcg)) > + memcg = rcu_dereference(current->active_memcg); > + else > + memcg = mem_cgroup_from_task(current); > + > + for (; memcg != root_mem_cgroup; memcg = parent_mem_cgroup(memcg)) { > + objcg = rcu_dereference(memcg->objcg); > + if (objcg && obj_cgroup_tryget(objcg)) > + break; > + } > + rcu_read_unlock(); > + > + return objcg; > +} Thanks for moving this here from one of the later patches, it helps understanding the life cycle of obj_cgroup better. > +int obj_cgroup_charge(struct obj_cgroup *objcg, gfp_t gfp, size_t size) > +{ > + struct mem_cgroup *memcg; > + unsigned int nr_pages, nr_bytes; > + int ret; > + > + if (consume_obj_stock(objcg, size)) > + return 0; > + > + rcu_read_lock(); > + memcg = obj_cgroup_memcg(objcg); > + css_get(&memcg->css); > + rcu_read_unlock(); > + > + nr_pages = size >> PAGE_SHIFT; > + nr_bytes = size & (PAGE_SIZE - 1); > + > + if (nr_bytes) > + nr_pages += 1; > + > + ret = __memcg_kmem_charge(memcg, gfp, nr_pages); If consume_obj_stock() fails because some other memcg is cached, should this try to consume the partial page in objcg->nr_charged_bytes before getting more pages? > + if (!ret && nr_bytes) > + refill_obj_stock(objcg, PAGE_SIZE - nr_bytes); This will put the cgroup into the cache if the allocation resulted in a partially free page. But if this was a page allocation, we may have objcg->nr_cache_bytes from a previous subpage allocation that we should probably put back into the stock. It's not a common case, I'm just trying to understand what objcg->nr_cache_bytes holds and when it does so. The rest of this patch looks good to me!