From patchwork Wed Sep 25 17:10:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kairui Song X-Patchwork-Id: 13812340 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25E20CCF9E7 for ; Wed, 25 Sep 2024 17:10:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A892F6B00BB; Wed, 25 Sep 2024 13:10:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A10906B00BC; Wed, 25 Sep 2024 13:10:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7EE776B00BD; Wed, 25 Sep 2024 13:10:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 522A56B00BB for ; Wed, 25 Sep 2024 13:10:48 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 16344140514 for ; Wed, 25 Sep 2024 17:10:48 +0000 (UTC) X-FDA: 82603900176.23.1551E74 Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) by imf11.hostedemail.com (Postfix) with ESMTP id 0A8034001F for ; Wed, 25 Sep 2024 17:10:45 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=FI5ia8DX; spf=pass (imf11.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727284185; h=from:from:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=91pGC7UhHYrZG8U8mTWFeIlYDkmoRgXvK0ZL4GQEbB8=; b=OCXF+eptjP7RspS2KDKcYsvD41PxNiGWhz4hgbsgvxqa8RwOS4WmUvqBvG7yihVsCdIZty +Pfs20Zrm4GWyBJYbVcuAsGpz7RFmYpD468C7N2OTnh5SBHXPYxQT2006MKXH6BFg0ysCH GOaMWMVqY7vsjzWExZLvZMzTtZlflkA= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=FI5ia8DX; spf=pass (imf11.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727284185; a=rsa-sha256; cv=none; b=1eekKgO+3u2ZiB45gvAP8FAq7fNzG5zdZOUVafDe/HfG0pOZCRft6JY0x9xvJRdweB0Tee Omdzsbub/w2QWLBNvJQxlOtY4clDEX/63uX/LeKMM/kedwgU1EPNc370bCfSt2wgPGjCiW jcd45BKxMCGUvizTf4X718a6Wv6GrSk= Received: by mail-pf1-f174.google.com with SMTP id d2e1a72fcca58-718e2855479so114981b3a.1 for ; Wed, 25 Sep 2024 10:10:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727284244; x=1727889044; darn=kvack.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=91pGC7UhHYrZG8U8mTWFeIlYDkmoRgXvK0ZL4GQEbB8=; b=FI5ia8DXyPbI6a/fOnb/yth6A6OPIfWzN+n8o2RI9QwCdonQXlZv6BbwN3OA+k5/p5 ViT8qWCKlt/RYfaRxg5DtYFe7p2twCyfJOppMUMWMnG8P72xvtZiPrKFfIp24sUAMfPj gvUExVZcoHq+KQyG0H/+17+Q/fbzOYj44kvZgajBbxKnfwd68YJgE/Fs7pAxMH+txjYC sSq7MLmd4pw/jE/Ws1CHRDEwbcx2ZVvLm30XOBqe4dQXnqk2Ks3chwk3h5Fr6hy8XYsm YEFAHKFzlOLVEZPEU+Xdh0SJVyQemIYUDWjgaDdgObSaJHghhLO5nI4vbKYauYvdGl/M HS4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727284244; x=1727889044; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=91pGC7UhHYrZG8U8mTWFeIlYDkmoRgXvK0ZL4GQEbB8=; b=GFJfvTHQ03P3QpwJCqC9KzAdnQLTPd8tm3+9NxWlLRqidxPciVQJhdKHszF5iFVq3V 1o1DJtASdYHULfywKgYH8RP/rdbfPVix52RNxyHGK9QMzqIsDcMCJrL44D7j2WluoDBZ yth4oSuFLVN7a3xOwxCsnzW4Pyvg8NS/twFpa78O6fqPwgBz4RXi8ogTFl+VXicP8ioK lerJxrjuADx3IwKyI2jsAFX+PEvPcD9zRPsXr7WttbqxuyBfVBsyqcRM3tWVEMxybjfB LPZyav6QwR6f1KqXBck781I4IguNkDJxMo3geyQVxBo+fVZKr1YbZuSY0a/ZIMXhuZcY dgKg== X-Gm-Message-State: AOJu0YxFDnYbRKwyanLQBRIio2W/wRJajMacJtPsNleF8erfBRMotG6D 0Z1jKfKnLsuvrj+kOWetR4zSKtpKgLHhT38J4pW61/vwa7eoSYWhChmnVKGk X-Google-Smtp-Source: AGHT+IF/P6uCxsPnmh+Iyr4oZKd26dI3Xr0+/R6nTvKKyRrKWSNV0eeDDmGrGJ+oOJ61ewGvGrEGLQ== X-Received: by 2002:a05:6a20:9c8e:b0:1cf:2d62:8584 with SMTP id adf61e73a8af0-1d4d4aaee26mr4723255637.11.1727284244134; Wed, 25 Sep 2024 10:10:44 -0700 (PDT) Received: from KASONG-MC4.tencent.com ([101.32.222.185]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71b032eb4f7sm2437900b3a.181.2024.09.25.10.10.40 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Wed, 25 Sep 2024 10:10:43 -0700 (PDT) From: Kairui Song To: linux-mm@kvack.org Cc: Andrew Morton , Matthew Wilcox , Johannes Weiner , Roman Gushchin , Waiman Long , Shakeel Butt , Michal Hocko , Chengming Zhou , Qi Zheng , Muchun Song , Kairui Song Subject: [PATCH v2 5/6] mm/list_lru: split the lock to per-cgroup scope Date: Thu, 26 Sep 2024 01:10:19 +0800 Message-ID: <20240925171020.32142-6-ryncsn@gmail.com> X-Mailer: git-send-email 2.46.1 In-Reply-To: <20240925171020.32142-1-ryncsn@gmail.com> References: <20240925171020.32142-1-ryncsn@gmail.com> Reply-To: Kairui Song MIME-Version: 1.0 X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 0A8034001F X-Stat-Signature: ajgwc6pcxd3u7i3s9peiwjxawkzenafg X-HE-Tag: 1727284245-719080 X-HE-Meta: U2FsdGVkX1+3qbVFOs7fqqw9CQL6k3HpM9B+PxUGDMzFC/0MhGUw82ErWMbWJDpf7A7P/yhmKJ3/Ih+Mi0LznMMnVMBYrT3Pp10ggz3WW+3awNqFrOkduxzsjhSpR2PtmH79HgkPHBzqGm5hM3kp4dP5RFS15GVmiYwfb2hN4mInUwV39E+NH9SV7g7GUZeYnWydCYTkoR3GmBirVWRKUUOjum2ehS8wtdyuti6XSVDf6sRUFDfiPOtMHMowdQjorkWCZ+O92eVrVrVgOT9vt2WEvvOMHyXlsjn04XEitN2ZdYtKcuIOnTYIGBc6WR/t95b8U8eXMrcZDFe2RYsQjWPGpW7OMUL1LcNJ9JdvcGAvjL11GucFiZKxvye/E1JDUxRAfoBKEmgRT2Kj/10LTHI11UvEfVrZaTfDSpv7SnjpUUDcf+AV+aHE/989m6YcxQM2CBH+N3fc2DNOwbDhxPy8LEbRscpRKQemLThI7XYfXiCV8c5afz88EEscuCCR4exqkLnzdIl9QsoOasLPgxSXLE14RQEI7cx9B680n2QE38LUnmf0JWcL6s9oyh9xGhYDINGf/o1xUw3rPxveHt/HLzaknU2xNTXaI3L6lmmeIcLTWe1swtis2CIt3tjmWOVLf8Rl3icozqvhiXUzQeoIw9T9JGykuMF1laD1To9VhyufwztZ5E4R2lI/ZvFAHCffBVRpINFUQ2n3lfwgAH4xeVYqgSjOvGmIow+H1227znBTHvH+c9yZyb25b3hJZPRCw0Y48Zwr7LnfjZTAintT8+VwuKaGQx4bOh848FXfdDu4OLvHpyfBGTHmzuXme/nUvOi6MGRuQkcxs2gu2p2usMzOiV+FUVH767RJ6i8KE47U0t+938OtfQX3gg7Gdd6k5vhm0ZMVOMajn2X4Ke5R1Eu0tIrvP6vYokIfG+zF5JHwpj3mOQz9nut73mW0PTZJEkUXDgpqtx4ygNg wTK9jT/+ +b7Y4z4OJj3PlT/xfnbc0p/EHeYoRt4hVf9q3igbylOGSQ/AxiioZmS5GI/w+J8dCrSrILHGmOFQ1gGzWSp+x7DykI2xZcsg2P2ll8+kavFpulGy5Uyat6FlTItBHZvVN2sooKzMwlmS3HzK6bPyusYLw2nmbBaPe5CEXXMYIKuJK3GEy3cuAM5kQXRD8RxDzf9geS5Sb15+5Vew2mSrBUk9TjOYOJnRvRyiUll4fU37m8Hyw8BRg1xtSMqQcxCxD3delBW4WNhUWxcHDGdQ45khgr+ZJHuTb4RcQ5gyBlFbnMxDasWLRAFgdZJzgf/4JomYaFm6mw9RlGrz1piMq6ScEV1eqVlWFlqHatzs7lq0tSlF6uxsaPzMT+iZftyfYIMOasLl5zlQVavIDtDq6/Pb58e3ElLYd/v8gFxAH9jt/c7aJgPR0m6LIcLnYxikH/raWNhXvOV2lS8nHYvApcTADES7t74/HxNd2C5tWolm5R5aD6TA4pWtzqRt+zYoy+PumRgRdtpOwlWcUe9KPTPqj349TweTdi2GZawE5r5IfnRe5iIx/JSamAB8nMosIoKb8 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: List-Subscribe: List-Unsubscribe: From: Kairui Song Currently, every list_lru has a per-node lock that protects adding, deletion, isolation, and reparenting of all list_lru_one instances belonging to this list_lru on this node. This lock contention is heavy when multiple cgroups modify the same list_lru. This lock can be split into per-cgroup scope to reduce contention. To achieve this, we need a stable list_lru_one for every cgroup. This commit adds a lock to each list_lru_one and introduced a helper function lock_list_lru_of_memcg, making it possible to pin the list_lru of a memcg. Then reworked the reparenting process. Reparenting will switch the list_lru_one instances one by one. By locking each instance and marking it dead using the nr_items counter, reparenting ensures that all items in the corresponding cgroup (on-list or not, because items have a stable cgroup, see below) will see the list_lru_one switch synchronously. Objcg reparent is also moved after list_lru reparent so items will have a stable mem cgroup until all list_lru_one instances are drained. The only caller that doesn't work the *_obj interfaces are direct calls to list_lru_{add,del}. But it's only used by zswap and that's also based on objcg, so it's fine. This also changes the bahaviour of the isolation function when LRU_RETRY or LRU_REMOVED_RETRY is returned, because now releasing the lock could unblock reparenting and free the list_lru_one, isolation function will have to return withoug re-lock the lru. prepare() { mkdir /tmp/test-fs modprobe brd rd_nr=1 rd_size=33554432 mkfs.xfs -f /dev/ram0 mount -t xfs /dev/ram0 /tmp/test-fs for i in $(seq 1 512); do mkdir "/tmp/test-fs/$i" for j in $(seq 1 10240); do echo TEST-CONTENT > "/tmp/test-fs/$i/$j" done & done; wait } do_test() { read_worker() { sleep 1 tar -cv "$1" &>/dev/null } read_in_all() { cd "/tmp/test-fs" && ls for i in $(seq 1 512); do (exec sh -c 'echo "$PPID"') > "/sys/fs/cgroup/benchmark/$i/cgroup.procs" read_worker "$i" & done; wait } for i in $(seq 1 512); do mkdir -p "/sys/fs/cgroup/benchmark/$i" done echo +memory > /sys/fs/cgroup/benchmark/cgroup.subtree_control echo 512M > /sys/fs/cgroup/benchmark/memory.max echo 3 > /proc/sys/vm/drop_caches time read_in_all } Above script simulates compression of small files in multiple cgroups with memory pressure. Run prepare() then do_test for 6 times: Before: real 0m7.762s user 0m11.340s sys 3m11.224s real 0m8.123s user 0m11.548s sys 3m2.549s real 0m7.736s user 0m11.515s sys 3m11.171s real 0m8.539s user 0m11.508s sys 3m7.618s real 0m7.928s user 0m11.349s sys 3m13.063s real 0m8.105s user 0m11.128s sys 3m14.313s After this commit (about ~15% faster): real 0m6.953s user 0m11.327s sys 2m42.912s real 0m7.453s user 0m11.343s sys 2m51.942s real 0m6.916s user 0m11.269s sys 2m43.957s real 0m6.894s user 0m11.528s sys 2m45.346s real 0m6.911s user 0m11.095s sys 2m43.168s real 0m6.773s user 0m11.518s sys 2m40.774s Signed-off-by: Kairui Song --- drivers/android/binder_alloc.c | 1 - fs/inode.c | 1 - fs/xfs/xfs_qm.c | 1 - include/linux/list_lru.h | 6 +- mm/list_lru.c | 215 +++++++++++++++++++-------------- mm/memcontrol.c | 7 +- mm/workingset.c | 1 - mm/zswap.c | 5 +- 8 files changed, 134 insertions(+), 103 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index b3acbc4174fb..86bbe40f4bcd 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -1106,7 +1106,6 @@ enum lru_status binder_alloc_free_page(struct list_head *item, mmput_async(mm); __free_page(page_to_free); - spin_lock(lock); return LRU_REMOVED_RETRY; err_invalid_vma: diff --git a/fs/inode.c b/fs/inode.c index 471ae4a31549..b95d80cb140b 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -932,7 +932,6 @@ static enum lru_status inode_lru_isolate(struct list_head *item, mm_account_reclaimed_pages(reap); } inode_unpin_lru_isolating(inode); - spin_lock(lru_lock); return LRU_RETRY; } diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index 7e2307921deb..665d26990b78 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -496,7 +496,6 @@ xfs_qm_dquot_isolate( trace_xfs_dqreclaim_busy(dqp); XFS_STATS_INC(dqp->q_mount, xs_qm_dqreclaim_misses); xfs_dqunlock(dqp); - spin_lock(lru_lock); return LRU_RETRY; } diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h index eba93f6511f3..10ba9a54d42c 100644 --- a/include/linux/list_lru.h +++ b/include/linux/list_lru.h @@ -32,6 +32,8 @@ struct list_lru_one { struct list_head list; /* may become negative during memcg reparenting */ long nr_items; + /* protects all fields above */ + spinlock_t lock; }; struct list_lru_memcg { @@ -41,11 +43,9 @@ struct list_lru_memcg { }; struct list_lru_node { - /* protects all lists on the node, including per cgroup */ - spinlock_t lock; /* global list, used for the root cgroup in cgroup aware lrus */ struct list_lru_one lru; - long nr_items; + atomic_long_t nr_items; } ____cacheline_aligned_in_smp; struct list_lru { diff --git a/mm/list_lru.c b/mm/list_lru.c index 172b16146e15..6d1c187d9019 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -61,18 +61,48 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx) } static inline struct list_lru_one * -list_lru_from_memcg(struct list_lru *lru, int nid, struct mem_cgroup *memcg) +lock_list_lru_of_memcg(struct list_lru *lru, int nid, struct mem_cgroup *memcg, + bool irq, bool skip_empty) { struct list_lru_one *l; + rcu_read_lock(); again: l = list_lru_from_memcg_idx(lru, nid, memcg_kmem_id(memcg)); - if (likely(l)) - return l; - - memcg = parent_mem_cgroup(memcg); + if (likely(l)) { + if (irq) + spin_lock_irq(&l->lock); + else + spin_lock(&l->lock); + if (likely(READ_ONCE(l->nr_items) != LONG_MIN)) { + WARN_ON(l->nr_items < 0); + rcu_read_unlock(); + return l; + } + if (irq) + spin_unlock_irq(&l->lock); + else + spin_unlock(&l->lock); + } + /* + * Caller may simply bail out if raced with reparenting or + * may iterate through the list_lru and expect empty slots. + */ + if (skip_empty) { + rcu_read_unlock(); + return NULL; + } VM_WARN_ON(!css_is_dying(&memcg->css)); + memcg = parent_mem_cgroup(memcg); goto again; } + +static inline void unlock_list_lru(struct list_lru_one *l, bool irq_off) +{ + if (irq_off) + spin_unlock_irq(&l->lock); + else + spin_unlock(&l->lock); +} #else static void list_lru_register(struct list_lru *lru) { @@ -99,31 +129,48 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx) } static inline struct list_lru_one * -list_lru_from_memcg(struct list_lru *lru, int nid, int idx) +lock_list_lru_of_memcg(struct list_lru *lru, int nid, struct mem_cgroup *memcg, + bool irq, bool skip_empty) { - return &lru->node[nid].lru; + struct list_lru_one *l = &lru->node[nid].lru; + + if (irq) + spin_lock_irq(&l->lock); + else + spin_lock(&l->lock); + + return l; +} + +static inline void unlock_list_lru(struct list_lru_one *l, bool irq_off) +{ + if (irq_off) + spin_unlock_irq(&l->lock); + else + spin_unlock(&l->lock); } #endif /* CONFIG_MEMCG */ /* The caller must ensure the memcg lifetime. */ bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid, - struct mem_cgroup *memcg) + struct mem_cgroup *memcg) { struct list_lru_node *nlru = &lru->node[nid]; struct list_lru_one *l; - spin_lock(&nlru->lock); + l = lock_list_lru_of_memcg(lru, nid, memcg, false, false); + if (!l) + return false; if (list_empty(item)) { - l = list_lru_from_memcg(lru, nid, memcg); list_add_tail(item, &l->list); /* Set shrinker bit if the first element was added */ if (!l->nr_items++) set_shrinker_bit(memcg, nid, lru_shrinker_id(lru)); - nlru->nr_items++; - spin_unlock(&nlru->lock); + unlock_list_lru(l, false); + atomic_long_inc(&nlru->nr_items); return true; } - spin_unlock(&nlru->lock); + unlock_list_lru(l, false); return false; } @@ -146,24 +193,23 @@ EXPORT_SYMBOL_GPL(list_lru_add_obj); /* The caller must ensure the memcg lifetime. */ bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid, - struct mem_cgroup *memcg) + struct mem_cgroup *memcg) { struct list_lru_node *nlru = &lru->node[nid]; struct list_lru_one *l; - - spin_lock(&nlru->lock); + l = lock_list_lru_of_memcg(lru, nid, memcg, false, false); + if (!l) + return false; if (!list_empty(item)) { - l = list_lru_from_memcg(lru, nid, memcg); list_del_init(item); l->nr_items--; - nlru->nr_items--; - spin_unlock(&nlru->lock); + unlock_list_lru(l, false); + atomic_long_dec(&nlru->nr_items); return true; } - spin_unlock(&nlru->lock); + unlock_list_lru(l, false); return false; } -EXPORT_SYMBOL_GPL(list_lru_del); bool list_lru_del_obj(struct list_lru *lru, struct list_head *item) { @@ -220,25 +266,24 @@ unsigned long list_lru_count_node(struct list_lru *lru, int nid) struct list_lru_node *nlru; nlru = &lru->node[nid]; - return nlru->nr_items; + return atomic_long_read(&nlru->nr_items); } EXPORT_SYMBOL_GPL(list_lru_count_node); static unsigned long -__list_lru_walk_one(struct list_lru *lru, int nid, int memcg_idx, +__list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg, list_lru_walk_cb isolate, void *cb_arg, - unsigned long *nr_to_walk) + unsigned long *nr_to_walk, bool irq_off) { struct list_lru_node *nlru = &lru->node[nid]; - struct list_lru_one *l; + struct list_lru_one *l = NULL; struct list_head *item, *n; unsigned long isolated = 0; restart: - l = list_lru_from_memcg_idx(lru, nid, memcg_idx); + l = lock_list_lru_of_memcg(lru, nid, memcg, irq_off, true); if (!l) - goto out; - + return isolated; list_for_each_safe(item, n, &l->list) { enum lru_status ret; @@ -250,19 +295,20 @@ __list_lru_walk_one(struct list_lru *lru, int nid, int memcg_idx, break; --*nr_to_walk; - ret = isolate(item, l, &nlru->lock, cb_arg); + ret = isolate(item, l, &l->lock, cb_arg); switch (ret) { + /* + * LRU_RETRY and LRU_REMOVED_RETRY will drop the lru lock, + * the list traversal will be invalid and have to restart from + * scratch. + */ + case LRU_RETRY: + goto restart; case LRU_REMOVED_RETRY: - assert_spin_locked(&nlru->lock); fallthrough; case LRU_REMOVED: isolated++; - nlru->nr_items--; - /* - * If the lru lock has been dropped, our list - * traversal is now invalid and so we have to - * restart from scratch. - */ + atomic_long_dec(&nlru->nr_items); if (ret == LRU_REMOVED_RETRY) goto restart; break; @@ -271,21 +317,15 @@ __list_lru_walk_one(struct list_lru *lru, int nid, int memcg_idx, break; case LRU_SKIP: break; - case LRU_RETRY: - /* - * The lru lock has been dropped, our list traversal is - * now invalid and so we have to restart from scratch. - */ - assert_spin_locked(&nlru->lock); - goto restart; case LRU_STOP: - assert_spin_locked(&nlru->lock); + assert_spin_locked(&l->lock); goto out; default: BUG(); } } out: + unlock_list_lru(l, irq_off); return isolated; } @@ -294,14 +334,8 @@ list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg, list_lru_walk_cb isolate, void *cb_arg, unsigned long *nr_to_walk) { - struct list_lru_node *nlru = &lru->node[nid]; - unsigned long ret; - - spin_lock(&nlru->lock); - ret = __list_lru_walk_one(lru, nid, memcg_kmem_id(memcg), isolate, - cb_arg, nr_to_walk); - spin_unlock(&nlru->lock); - return ret; + return __list_lru_walk_one(lru, nid, memcg, isolate, + cb_arg, nr_to_walk, false); } EXPORT_SYMBOL_GPL(list_lru_walk_one); @@ -310,14 +344,8 @@ list_lru_walk_one_irq(struct list_lru *lru, int nid, struct mem_cgroup *memcg, list_lru_walk_cb isolate, void *cb_arg, unsigned long *nr_to_walk) { - struct list_lru_node *nlru = &lru->node[nid]; - unsigned long ret; - - spin_lock_irq(&nlru->lock); - ret = __list_lru_walk_one(lru, nid, memcg_kmem_id(memcg), isolate, - cb_arg, nr_to_walk); - spin_unlock_irq(&nlru->lock); - return ret; + return __list_lru_walk_one(lru, nid, memcg, isolate, + cb_arg, nr_to_walk, true); } unsigned long list_lru_walk_node(struct list_lru *lru, int nid, @@ -332,16 +360,21 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid, #ifdef CONFIG_MEMCG if (*nr_to_walk > 0 && list_lru_memcg_aware(lru)) { struct list_lru_memcg *mlru; + struct mem_cgroup *memcg; unsigned long index; xa_for_each(&lru->xa, index, mlru) { - struct list_lru_node *nlru = &lru->node[nid]; - - spin_lock(&nlru->lock); - isolated += __list_lru_walk_one(lru, nid, index, + rcu_read_lock(); + memcg = mem_cgroup_from_id(index); + if (!mem_cgroup_tryget(memcg)) { + rcu_read_unlock(); + continue; + } + rcu_read_unlock(); + isolated += __list_lru_walk_one(lru, nid, memcg, isolate, cb_arg, - nr_to_walk); - spin_unlock(&nlru->lock); + nr_to_walk, false); + mem_cgroup_put(memcg); if (*nr_to_walk <= 0) break; @@ -353,14 +386,19 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid, } EXPORT_SYMBOL_GPL(list_lru_walk_node); -static void init_one_lru(struct list_lru_one *l) +static void init_one_lru(struct list_lru *lru, struct list_lru_one *l) { INIT_LIST_HEAD(&l->list); + spin_lock_init(&l->lock); l->nr_items = 0; +#ifdef CONFIG_LOCKDEP + if (lru->key) + lockdep_set_class(&l->lock, lru->key); +#endif } #ifdef CONFIG_MEMCG -static struct list_lru_memcg *memcg_init_list_lru_one(gfp_t gfp) +static struct list_lru_memcg *memcg_init_list_lru_one(struct list_lru *lru, gfp_t gfp) { int nid; struct list_lru_memcg *mlru; @@ -370,7 +408,7 @@ static struct list_lru_memcg *memcg_init_list_lru_one(gfp_t gfp) return NULL; for_each_node(nid) - init_one_lru(&mlru->node[nid]); + init_one_lru(lru, &mlru->node[nid]); return mlru; } @@ -398,28 +436,27 @@ static void memcg_destroy_list_lru(struct list_lru *lru) xas_unlock_irq(&xas); } -static void memcg_reparent_list_lru_node(struct list_lru *lru, int nid, - struct list_lru_one *src, - struct mem_cgroup *dst_memcg) +static void memcg_reparent_list_lru_one(struct list_lru *lru, int nid, + struct list_lru_one *src, + struct mem_cgroup *dst_memcg) { - struct list_lru_node *nlru = &lru->node[nid]; + int dst_idx = dst_memcg->kmemcg_id; struct list_lru_one *dst; - /* - * Since list_lru_{add,del} may be called under an IRQ-safe lock, - * we have to use IRQ-safe primitives here to avoid deadlock. - */ - spin_lock_irq(&nlru->lock); - dst = list_lru_from_memcg_idx(lru, nid, memcg_kmem_id(dst_memcg)); + spin_lock_irq(&src->lock); + dst = list_lru_from_memcg_idx(lru, nid, dst_idx); + spin_lock_nested(&dst->lock, SINGLE_DEPTH_NESTING); list_splice_init(&src->list, &dst->list); - if (src->nr_items) { dst->nr_items += src->nr_items; set_shrinker_bit(dst_memcg, nid, lru_shrinker_id(lru)); - src->nr_items = 0; } - spin_unlock_irq(&nlru->lock); + /* Mark the list_lru_one dead */ + src->nr_items = LONG_MIN; + + spin_unlock(&dst->lock); + spin_unlock_irq(&src->lock); } void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *parent) @@ -448,7 +485,7 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren * safe to reparent. */ for_each_node(i) - memcg_reparent_list_lru_node(lru, i, &mlru->node[i], parent); + memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent); /* * Here all list_lrus corresponding to the cgroup are guaranteed @@ -497,7 +534,7 @@ int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru, parent = parent_mem_cgroup(pos); } - mlru = memcg_init_list_lru_one(gfp); + mlru = memcg_init_list_lru_one(lru, gfp); if (!mlru) return -ENOMEM; xas_set(&xas, pos->kmemcg_id); @@ -544,14 +581,8 @@ int __list_lru_init(struct list_lru *lru, bool memcg_aware, struct shrinker *shr if (!lru->node) return -ENOMEM; - for_each_node(i) { - spin_lock_init(&lru->node[i].lock); -#ifdef CONFIG_LOCKDEP - if (lru->key) - lockdep_set_class(&lru->node[i].lock, lru->key); -#endif - init_one_lru(&lru->node[i].lru); - } + for_each_node(i) + init_one_lru(lru, &lru->node[i].lru); memcg_init_list_lru(lru, memcg_aware); list_lru_register(lru); diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 8e90aa026c47..f421dfcfe8a1 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3098,8 +3098,13 @@ static void memcg_offline_kmem(struct mem_cgroup *memcg) if (!parent) parent = root_mem_cgroup; - memcg_reparent_objcgs(memcg, parent); memcg_reparent_list_lrus(memcg, parent); + + /* + * Objcg's reparenting must be after list_lru's, make sure list_lru + * helpers won't use parent's list_lru until child is drained. + */ + memcg_reparent_objcgs(memcg, parent); } #ifdef CONFIG_CGROUP_WRITEBACK diff --git a/mm/workingset.c b/mm/workingset.c index df3937c5eedc..8c4b6738dcad 100644 --- a/mm/workingset.c +++ b/mm/workingset.c @@ -777,7 +777,6 @@ static enum lru_status shadow_lru_isolate(struct list_head *item, ret = LRU_REMOVED_RETRY; out: cond_resched(); - spin_lock_irq(lru_lock); return ret; } diff --git a/mm/zswap.c b/mm/zswap.c index 95a28301b7cd..cb37a25fd00c 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -706,9 +706,9 @@ static void zswap_lru_add(struct list_lru *list_lru, struct zswap_entry *entry) * Note that it is safe to use rcu_read_lock() here, even in the face of * concurrent memcg offlining: * - * 1. list_lru_add() is called before list_lru_memcg is erased. The + * 1. list_lru_add() is called before list_lru_one is dead. The * new entry will be reparented to memcg's parent's list_lru. - * 2. list_lru_add() is called after list_lru_memcg is erased. The + * 2. list_lru_add() is called after list_lru_one is dead. The * new entry will be added directly to memcg's parent's list_lru. * * Similar reasoning holds for list_lru_del(). @@ -1173,7 +1173,6 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o zswap_written_back_pages++; } - spin_lock(lock); return ret; }