Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220228122126.37293-1-songmuchun@bytedance.com>
 <20220228122126.37293-13-songmuchun@bytedance.com> <164869718565.25542.15818719940772238394@noble.neil.brown.name>
In-Reply-To: <164869718565.25542.15818719940772238394@noble.neil.brown.name>
From:   Muchun Song <songmuchun@bytedance.com>
Date:   Thu, 31 Mar 2022 11:52:50 +0800
Message-ID: <CAMZfGtUSA9f3k9jF5U-y+NVt8cpmB9_mk1F9-vmm4JOuWFF_Bw@mail.gmail.com>
Subject: Re: [PATCH v6 12/16] mm: list_lru: replace linear array with xarray
To:     NeilBrown <neilb@suse.de>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Vladimir Davydov <vdavydov.dev@gmail.com>,
        Shakeel Butt <shakeelb@google.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Yang Shi <shy828301@gmail.com>, Alex Shi <alexs@kernel.org>,
        Wei Yang <richard.weiyang@gmail.com>,
        Dave Chinner <david@fromorbit.com>,
        trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
        jaegeuk@kernel.org, chao@kernel.org,
        Kari Argillander <kari.argillander@gmail.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        linux-nfs@vger.kernel.org, Qi Zheng <zhengqi.arch@bytedance.com>,
        Xiongchun duan <duanxiongchun@bytedance.com>,
        Fam Zheng <fam.zheng@bytedance.com>,
        Muchun Song <smuchun@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 31, 2022 at 11:26 AM NeilBrown <neilb@suse.de> wrote:
>
> On Mon, 28 Feb 2022, 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.
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>
> Hi,
>  I've just tried some simple testing on NFS (xfstests generic/???) and
>  it reliably crashes in this code.
>  Specifically in list_lru_add(), list_lru_from_kmem() returns NULL,
>  which results in a NULL deref.
>  list_lru_from_kmem() returns NULL because an xa_load() returns NULL.

Did you test with the patch [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=ae085d7f9365de7da27ab5c0d16b12d51ea7fca9

>
>  The patch doesn't make any sense to me.  It replaces an array of
>  structures with an xarray, which is an array of pointers.
>  It seems to assume that all the pointers in the array get magically
>  allocated to the required structures.  I certainly cannot find when
>  the 'struct list_lru_node' structures get allocated.  So xa_load() will
>  *always* return NULL in this code.

struct list_lru_node is allocated via kmem_cache_alloc_lru().

>
>  I can provide more details of the failure stack if needed, but I doubt
>  that would be necessary.
>

If the above fix cannot fix your issue, would you mind providing
the .config and stack trace?

Thanks for your report.