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=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 58FF4C4708D for ; Fri, 28 May 2021 08:04:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 35319610C9 for ; Fri, 28 May 2021 08:04:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235763AbhE1IGV (ORCPT ); Fri, 28 May 2021 04:06:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59330 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230187AbhE1IGU (ORCPT ); Fri, 28 May 2021 04:06:20 -0400 Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C38E2C061760 for ; Fri, 28 May 2021 01:04:44 -0700 (PDT) Received: by mail-pg1-x531.google.com with SMTP id t193so1950452pgb.4 for ; Fri, 28 May 2021 01:04:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JhQPjLykR01Qp+NSeFFJI6fs6yER95zPQTTz3vzX0YM=; b=olovBvIActNmkePfVD5Pug5meGIIltnoam/DS0feX69PrH5AWeqb/+XlZ1XTzUaESW ZcTXp0PKgC71S5Rp1U8usKjBcZdSfrRdFFeNZ0dWNZsaMFmYJycBNUmWyeCv3cCuP2IR Ibx6sRQElSaZkYu0Y3JuU0j7Ou+928szLcOwGMobKVEEqq762kkW95+nX8+7hb4zotV/ obpKtG1JfIq3N3ZNFOO0+ZGUm3GIIb8BtC1tLsav+PcMHC1gqwUivohdBa/j9zqxQouL dAoLqxJem+D6o2Py5pPf2IvpM7Pv9MR1xIUhY8RFGmRsWEWaBEW7VaqgKe1dOeAzfcZE kN7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JhQPjLykR01Qp+NSeFFJI6fs6yER95zPQTTz3vzX0YM=; b=t3e82YBXyz2BhhABMeg7VLO8Tc7+Kt8Vskh23tls64IeKed12bxU6QUflmpmHLaguO 4OpuPZgJ8StUhw4qYp6eNVd+KgTi58xlz14T5REf+h+qKEamAhc7dUlCrUrLpdgN67jg NbX43PnfbOiMefjpf6rkWURdB94H9zl5WIep2D4pL/STYeNoexE6yzvlC2KQNEjz8bky FheMaSSNL/IUXwvw1alGEdOjfZf0jbEK5E8BhexgGy66E7S6cp8E1nWf8Ph/QIGBbOkd I5SsYQzaYOOUj/Q9YLpcnsE0KCcN+jmUsF9VeVkY1vkx8CjkOD3N9tuav0TgWZkaCCZD AGtQ== X-Gm-Message-State: AOAM530Co7Q+8Y2R65kcxAvP6jfrGObfDmgwhdiBVXxh8KCZOJgR3jT8 XGc8bmy/fwt7Pl7gNIAekxqMCSFFBSKgGk+Z4j5jzQ== X-Google-Smtp-Source: ABdhPJzIQiwrZqUIU/7odmrvbjoVqWScYQNEHtklusDGA07cMOOcTJxGQSELDjMuvbe6w5bzfvVdGVzjsS64c1m7uyw= X-Received: by 2002:a62:7b07:0:b029:2e3:b540:707f with SMTP id w7-20020a627b070000b02902e3b540707fmr2602202pfc.59.1622189084220; Fri, 28 May 2021 01:04:44 -0700 (PDT) MIME-Version: 1.0 References: <20210527062148.9361-1-songmuchun@bytedance.com> <20210527062148.9361-18-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Fri, 28 May 2021 16:04:07 +0800 Message-ID: Subject: Re: [External] Re: [PATCH v2 17/21] mm: list_lru: replace linear array with xarray To: Matthew Wilcox Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Vladimir Davydov , Shakeel Butt , Roman Gushchin , Yang Shi , Alex Shi , Wei Yang , Dave Chinner , trond.myklebust@hammerspace.com, anna.schumaker@netapp.com, linux-fsdevel , LKML , Linux Memory Management List , linux-nfs@vger.kernel.org, zhengqi.arch@bytedance.com, Xiongchun duan , fam.zheng@bytedance.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 28, 2021 at 11:43 AM Muchun Song wrote: > > On Thu, May 27, 2021 at 8:08 PM Matthew Wilcox wrote: > > > > On Thu, May 27, 2021 at 02:21:44PM +0800, Muchun Song wrote: > > > If we run 10k containers in the system, the size of the > > > list_lru_memcg->lrus can be ~96KB per list_lru. When we decrease the > > > number containers, the size of the array will not be shrinked. It is > > > not scalable. The xarray is a good choice for this case. We can save > > > a lot of memory when there are tens of thousands continers in the > > > system. If we use xarray, we also can remove the logic code of > > > resizing array, which can simplify the code. > > > > I am all for this, in concept. Some thoughts below ... > > > > > @@ -56,10 +51,8 @@ struct list_lru { > > > #ifdef CONFIG_MEMCG_KMEM > > > struct list_head list; > > > int shrinker_id; > > > - /* protects ->memcg_lrus->lrus[i] */ > > > - spinlock_t lock; > > > /* for cgroup aware lrus points to per cgroup lists, otherwise NULL */ > > > - struct list_lru_memcg __rcu *memcg_lrus; > > > + struct xarray *xa; > > > #endif > > > > Normally, we embed an xarray in its containing structure instead of > > allocating it. It's only a pointer, int and spinlock, so generally > > 16 bytes, as opposed to the 8 bytes for the pointer and a 16 byte > > allocation. There is a minor wrinkle in that currently 'NULL' is > > used to indicate "is not cgroup aware". Maybe there's another way > > to indicate that? > > Sure. I can drop patch 8 in this series. In that case, we can use > ->memcg_aware to indicate that. > > > > > > > @@ -51,22 +51,12 @@ static int lru_shrinker_id(struct list_lru *lru) > > > static inline struct list_lru_one * > > > list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx) > > > { > > > - struct list_lru_memcg *memcg_lrus; > > > - struct list_lru_node *nlru = &lru->node[nid]; > > > + if (list_lru_memcg_aware(lru) && idx >= 0) { > > > + struct list_lru_per_memcg *mlru = xa_load(lru->xa, idx); > > > > > > - /* > > > - * Either lock or RCU protects the array of per cgroup lists > > > - * from relocation (see memcg_update_list_lru). > > > - */ > > > - memcg_lrus = rcu_dereference_check(lru->memcg_lrus, > > > - lockdep_is_held(&nlru->lock)); > > > - if (memcg_lrus && idx >= 0) { > > > - struct list_lru_per_memcg *mlru; > > > - > > > - mlru = rcu_dereference_check(memcg_lrus->lrus[idx], true); > > > return mlru ? &mlru->nodes[nid] : NULL; > > > } > > > - return &nlru->lru; > > > + return &lru->node[nid].lru; > > > } > > > > ... perhaps we move the xarray out from under the #ifdef and use index 0 > > for non-memcg-aware lrus? The XArray is specially optimised for arrays > > which only have one entry at 0. > > Sounds like a good idea. I can do a try. I have thought more about this. If we do this, we need to allocate a list_lru_per_memcg structure for the root memcg. Since the structure of list_lru_node already aligns with cache line size. From this point of view, this just wastes memory. We do not gain anything. Right? Another approach is introducing a new structure of list_lru_nodes, which is described as following. struct list_lru_nodes { struct list_lru_node nodes[0]; }; Then we insert struct list_lru_nodes to the XArray (index == 0). There will be two different types in the XArray. If index == 0, the xa_load() returns list_lru_nodes pointer, otherwise, it returns list_lru_per_memcg pointer. So list_lru_from_memcg_idx() still need to handle different cases. It looks like both approaches do not have any obvious advantages. What do you think about this, Mattew? > > > > > > int list_lru_memcg_alloc(struct list_lru *lru, struct mem_cgroup *memcg, gfp_t gfp) > > > { > > > + XA_STATE(xas, lru->xa, 0); > > > unsigned long flags; > > > - struct list_lru_memcg *memcg_lrus; > > > - int i; > > > + int i, ret = 0; > > > > > > struct list_lru_memcg_table { > > > struct list_lru_per_memcg *mlru; > > > @@ -601,22 +522,45 @@ int list_lru_memcg_alloc(struct list_lru *lru, struct mem_cgroup *memcg, gfp_t g > > > } > > > } > > > > > > - spin_lock_irqsave(&lru->lock, flags); > > > - memcg_lrus = rcu_dereference_protected(lru->memcg_lrus, true); > > > + xas_lock_irqsave(&xas, flags); > > > while (i--) { > > > int index = memcg_cache_id(table[i].memcg); > > > struct list_lru_per_memcg *mlru = table[i].mlru; > > > > > > - if (index < 0 || rcu_dereference_protected(memcg_lrus->lrus[index], true)) > > > + xas_set(&xas, index); > > > +retry: > > > + if (unlikely(index < 0 || ret || xas_load(&xas))) { > > > kfree(mlru); > > > - else > > > - rcu_assign_pointer(memcg_lrus->lrus[index], mlru); > > > + } else { > > > + ret = xa_err(xas_store(&xas, mlru)); > > > > This is mixing advanced and normal XArray concepts ... sorry to have > > confused you. I think what you meant to do here was: > > > > xas_store(&xas, mlru); > > ret = xas_error(&xas); > > Sure. Thanks for pointing it out. It's my bad usage. > > > > > Or you can avoid introducing 'ret' at all, and keep your errors in the > > xa_state. You're kind of mirroring the xa_state errors into 'ret' > > anyway, so that seems easier to understand? > > Make sense. I will do this in the next version. Thanks for your > all suggestions. > > > > > > - memcg_id = memcg_alloc_cache_id(); > > > + memcg_id = ida_simple_get(&memcg_cache_ida, 0, MEMCG_CACHES_MAX_SIZE, > > > + GFP_KERNEL); > > > > memcg_id = ida_alloc_max(&memcg_cache_ida, > > MEMCG_CACHES_MAX_SIZE - 1, GFP_KERNEL); > > > > ... although i think there's actually a fencepost error, and this really > > should be MEMCG_CACHES_MAX_SIZE. > > Totally agree. I have fixed this issue in patch 19. > > > > > > objcg = obj_cgroup_alloc(); > > > if (!objcg) { > > > - memcg_free_cache_id(memcg_id); > > > + ida_simple_remove(&memcg_cache_ida, memcg_id); > > > > ida_free(&memcg_cache_ida, memcg_id); > > I Will update to this new API. > > > 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=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 2883BC4708F for ; Fri, 28 May 2021 08:04:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 709D5613B6 for ; Fri, 28 May 2021 08:04:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 709D5613B6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 872246B006C; Fri, 28 May 2021 04:04:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7E52B6B006E; Fri, 28 May 2021 04:04:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2B65B6B0070; Fri, 28 May 2021 04:04:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id DA6416B006C for ; Fri, 28 May 2021 04:04:46 -0400 (EDT) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 67BE8824999B for ; Fri, 28 May 2021 08:04:46 +0000 (UTC) X-FDA: 78189903372.11.468D28C Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) by imf03.hostedemail.com (Postfix) with ESMTP id 1F12FC007774 for ; Fri, 28 May 2021 08:04:37 +0000 (UTC) Received: by mail-pg1-f178.google.com with SMTP id l70so1962644pga.1 for ; Fri, 28 May 2021 01:04:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JhQPjLykR01Qp+NSeFFJI6fs6yER95zPQTTz3vzX0YM=; b=olovBvIActNmkePfVD5Pug5meGIIltnoam/DS0feX69PrH5AWeqb/+XlZ1XTzUaESW ZcTXp0PKgC71S5Rp1U8usKjBcZdSfrRdFFeNZ0dWNZsaMFmYJycBNUmWyeCv3cCuP2IR Ibx6sRQElSaZkYu0Y3JuU0j7Ou+928szLcOwGMobKVEEqq762kkW95+nX8+7hb4zotV/ obpKtG1JfIq3N3ZNFOO0+ZGUm3GIIb8BtC1tLsav+PcMHC1gqwUivohdBa/j9zqxQouL dAoLqxJem+D6o2Py5pPf2IvpM7Pv9MR1xIUhY8RFGmRsWEWaBEW7VaqgKe1dOeAzfcZE kN7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JhQPjLykR01Qp+NSeFFJI6fs6yER95zPQTTz3vzX0YM=; b=HdImUXic26pJO0K72daFVieJzw2HC05phcEz7/8DBSo5H+hDMp1n/CD+NSELAhjzaW idi0cHoJuNp8JJrhorD1yNnrs0iwkn13ZBjEFif4QyY++N+DO8ay0K/tjjDN9l3K1p3j tOErNTLhchxJZ5UBsJJMNwdx8X6q6+bUGDHXsOccqp9VZ9klCV0vh1SuAzSa4c3K+3dd gaplo0zin2Xc0wCApEz5rn2eK/aerbECbzRXsrPPLf4IxJbBuJ0dk8eAPR7R4sNbdftW h6LvZC3RL7AEiVM5K9jkJlDL3fry5q3Kt/calWusmF/9qVPXwKVn9oSKiphMxntQDOLI Cs0w== X-Gm-Message-State: AOAM533goPS+Chc7aWX5KEQMcBWIvvllL9tUlyYslg8Cu8tYKcWT++LB sDzB0mVGhZ+/B6ZaXWZ5sgIH7XZ2vGRkzK42SxYTAA== X-Google-Smtp-Source: ABdhPJzIQiwrZqUIU/7odmrvbjoVqWScYQNEHtklusDGA07cMOOcTJxGQSELDjMuvbe6w5bzfvVdGVzjsS64c1m7uyw= X-Received: by 2002:a62:7b07:0:b029:2e3:b540:707f with SMTP id w7-20020a627b070000b02902e3b540707fmr2602202pfc.59.1622189084220; Fri, 28 May 2021 01:04:44 -0700 (PDT) MIME-Version: 1.0 References: <20210527062148.9361-1-songmuchun@bytedance.com> <20210527062148.9361-18-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Fri, 28 May 2021 16:04:07 +0800 Message-ID: Subject: Re: [External] Re: [PATCH v2 17/21] mm: list_lru: replace linear array with xarray To: Matthew Wilcox Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Vladimir Davydov , Shakeel Butt , Roman Gushchin , Yang Shi , Alex Shi , Wei Yang , Dave Chinner , trond.myklebust@hammerspace.com, anna.schumaker@netapp.com, linux-fsdevel , LKML , Linux Memory Management List , linux-nfs@vger.kernel.org, zhengqi.arch@bytedance.com, Xiongchun duan , fam.zheng@bytedance.com Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=bytedance-com.20150623.gappssmtp.com header.s=20150623 header.b=olovBvIA; spf=pass (imf03.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.215.178 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 1F12FC007774 X-Stat-Signature: aajw5gicifkf4wkk9p1cqtdda6xca8pj X-HE-Tag: 1622189077-14495 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 Fri, May 28, 2021 at 11:43 AM Muchun Song wrote: > > On Thu, May 27, 2021 at 8:08 PM Matthew Wilcox wrote: > > > > On Thu, May 27, 2021 at 02:21:44PM +0800, Muchun Song wrote: > > > If we run 10k containers in the system, the size of the > > > list_lru_memcg->lrus can be ~96KB per list_lru. When we decrease the > > > number containers, the size of the array will not be shrinked. It is > > > not scalable. The xarray is a good choice for this case. We can save > > > a lot of memory when there are tens of thousands continers in the > > > system. If we use xarray, we also can remove the logic code of > > > resizing array, which can simplify the code. > > > > I am all for this, in concept. Some thoughts below ... > > > > > @@ -56,10 +51,8 @@ struct list_lru { > > > #ifdef CONFIG_MEMCG_KMEM > > > struct list_head list; > > > int shrinker_id; > > > - /* protects ->memcg_lrus->lrus[i] */ > > > - spinlock_t lock; > > > /* for cgroup aware lrus points to per cgroup lists, otherwise NULL */ > > > - struct list_lru_memcg __rcu *memcg_lrus; > > > + struct xarray *xa; > > > #endif > > > > Normally, we embed an xarray in its containing structure instead of > > allocating it. It's only a pointer, int and spinlock, so generally > > 16 bytes, as opposed to the 8 bytes for the pointer and a 16 byte > > allocation. There is a minor wrinkle in that currently 'NULL' is > > used to indicate "is not cgroup aware". Maybe there's another way > > to indicate that? > > Sure. I can drop patch 8 in this series. In that case, we can use > ->memcg_aware to indicate that. > > > > > > > @@ -51,22 +51,12 @@ static int lru_shrinker_id(struct list_lru *lru) > > > static inline struct list_lru_one * > > > list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx) > > > { > > > - struct list_lru_memcg *memcg_lrus; > > > - struct list_lru_node *nlru = &lru->node[nid]; > > > + if (list_lru_memcg_aware(lru) && idx >= 0) { > > > + struct list_lru_per_memcg *mlru = xa_load(lru->xa, idx); > > > > > > - /* > > > - * Either lock or RCU protects the array of per cgroup lists > > > - * from relocation (see memcg_update_list_lru). > > > - */ > > > - memcg_lrus = rcu_dereference_check(lru->memcg_lrus, > > > - lockdep_is_held(&nlru->lock)); > > > - if (memcg_lrus && idx >= 0) { > > > - struct list_lru_per_memcg *mlru; > > > - > > > - mlru = rcu_dereference_check(memcg_lrus->lrus[idx], true); > > > return mlru ? &mlru->nodes[nid] : NULL; > > > } > > > - return &nlru->lru; > > > + return &lru->node[nid].lru; > > > } > > > > ... perhaps we move the xarray out from under the #ifdef and use index 0 > > for non-memcg-aware lrus? The XArray is specially optimised for arrays > > which only have one entry at 0. > > Sounds like a good idea. I can do a try. I have thought more about this. If we do this, we need to allocate a list_lru_per_memcg structure for the root memcg. Since the structure of list_lru_node already aligns with cache line size. From this point of view, this just wastes memory. We do not gain anything. Right? Another approach is introducing a new structure of list_lru_nodes, which is described as following. struct list_lru_nodes { struct list_lru_node nodes[0]; }; Then we insert struct list_lru_nodes to the XArray (index == 0). There will be two different types in the XArray. If index == 0, the xa_load() returns list_lru_nodes pointer, otherwise, it returns list_lru_per_memcg pointer. So list_lru_from_memcg_idx() still need to handle different cases. It looks like both approaches do not have any obvious advantages. What do you think about this, Mattew? > > > > > > int list_lru_memcg_alloc(struct list_lru *lru, struct mem_cgroup *memcg, gfp_t gfp) > > > { > > > + XA_STATE(xas, lru->xa, 0); > > > unsigned long flags; > > > - struct list_lru_memcg *memcg_lrus; > > > - int i; > > > + int i, ret = 0; > > > > > > struct list_lru_memcg_table { > > > struct list_lru_per_memcg *mlru; > > > @@ -601,22 +522,45 @@ int list_lru_memcg_alloc(struct list_lru *lru, struct mem_cgroup *memcg, gfp_t g > > > } > > > } > > > > > > - spin_lock_irqsave(&lru->lock, flags); > > > - memcg_lrus = rcu_dereference_protected(lru->memcg_lrus, true); > > > + xas_lock_irqsave(&xas, flags); > > > while (i--) { > > > int index = memcg_cache_id(table[i].memcg); > > > struct list_lru_per_memcg *mlru = table[i].mlru; > > > > > > - if (index < 0 || rcu_dereference_protected(memcg_lrus->lrus[index], true)) > > > + xas_set(&xas, index); > > > +retry: > > > + if (unlikely(index < 0 || ret || xas_load(&xas))) { > > > kfree(mlru); > > > - else > > > - rcu_assign_pointer(memcg_lrus->lrus[index], mlru); > > > + } else { > > > + ret = xa_err(xas_store(&xas, mlru)); > > > > This is mixing advanced and normal XArray concepts ... sorry to have > > confused you. I think what you meant to do here was: > > > > xas_store(&xas, mlru); > > ret = xas_error(&xas); > > Sure. Thanks for pointing it out. It's my bad usage. > > > > > Or you can avoid introducing 'ret' at all, and keep your errors in the > > xa_state. You're kind of mirroring the xa_state errors into 'ret' > > anyway, so that seems easier to understand? > > Make sense. I will do this in the next version. Thanks for your > all suggestions. > > > > > > - memcg_id = memcg_alloc_cache_id(); > > > + memcg_id = ida_simple_get(&memcg_cache_ida, 0, MEMCG_CACHES_MAX_SIZE, > > > + GFP_KERNEL); > > > > memcg_id = ida_alloc_max(&memcg_cache_ida, > > MEMCG_CACHES_MAX_SIZE - 1, GFP_KERNEL); > > > > ... although i think there's actually a fencepost error, and this really > > should be MEMCG_CACHES_MAX_SIZE. > > Totally agree. I have fixed this issue in patch 19. > > > > > > objcg = obj_cgroup_alloc(); > > > if (!objcg) { > > > - memcg_free_cache_id(memcg_id); > > > + ida_simple_remove(&memcg_cache_ida, memcg_id); > > > > ida_free(&memcg_cache_ida, memcg_id); > > I Will update to this new API. > > >