[Deepin-Kernel-SIG] [linux 6.6.y] [Upstream] zswap/list_lru: restore list_lru_add/list_lru_del for KAPI - #2030
Conversation
Reviewer's GuideRestores the legacy two-argument list_lru_add/list_lru_del kernel API for out-of-tree modules while keeping the newer memcg-aware internals, and updates zswap and list_lru internals to use new internal helpers and a simplified memcg accessor. Sequence diagram for updated list_lru_add/list_lru_del call pathssequenceDiagram
actor OutOfTreeModule
participant zswap
participant list_lru_API
participant list_lru_add_obj
participant __list_lru_add
participant list_lru_del_obj
participant __list_lru_del
OutOfTreeModule->>list_lru_API: list_lru_add(lru, item)
list_lru_API->>list_lru_add_obj: list_lru_add_obj(lru, item)
alt list_lru_memcg_aware(lru)
list_lru_add_obj->>__list_lru_add: __list_lru_add(lru, item, nid, memcg_from_slab_obj)
else
list_lru_add_obj->>__list_lru_add: __list_lru_add(lru, item, nid, NULL)
end
OutOfTreeModule->>list_lru_API: list_lru_del(lru, item)
list_lru_API->>list_lru_del_obj: list_lru_del_obj(lru, item)
alt list_lru_memcg_aware(lru)
list_lru_del_obj->>__list_lru_del: __list_lru_del(lru, item, nid, memcg_from_slab_obj)
else
list_lru_del_obj->>__list_lru_del: __list_lru_del(lru, item, nid, NULL)
end
zswap->>__list_lru_add: __list_lru_add(list_lru, entry_lru, nid, mem_cgroup_from_entry)
zswap->>__list_lru_del: __list_lru_del(list_lru, entry_lru, nid, mem_cgroup_from_entry)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- If __list_lru_add/__list_lru_del are intended only for in-tree callers, consider moving their declarations out of the public list_lru.h header (or clearly marking them as internal) to avoid encouraging use by out-of-tree modules despite the leading underscores.
- The new legacy list_lru_add/list_lru_del wrappers rely on mem_cgroup_from_slab_obj via list_lru_add_obj/list_lru_del_obj; if there are known legacy callers that expect non-slab items or different nid selection, it may be worth double-checking and documenting any behavioral differences from the old KAPI.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If __list_lru_add/__list_lru_del are intended only for in-tree callers, consider moving their declarations out of the public list_lru.h header (or clearly marking them as internal) to avoid encouraging use by out-of-tree modules despite the leading underscores.
- The new legacy list_lru_add/list_lru_del wrappers rely on mem_cgroup_from_slab_obj via list_lru_add_obj/list_lru_del_obj; if there are known legacy callers that expect non-slab items or different nid selection, it may be worth double-checking and documenting any behavioral differences from the old KAPI.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR restores the legacy two-argument list_lru_add() / list_lru_del() KAPI for out-of-tree users while keeping in-tree callers (notably zswap) on new internal memcg-aware helpers.
Changes:
- Introduces internal
__list_lru_add()/__list_lru_del()APIs that take(nid, memcg)for in-tree callers. - Reintroduces legacy two-argument
list_lru_add()/list_lru_del()wrappers (exported) for KAPI compatibility. - Updates zswap to call
__list_lru_add()/__list_lru_del()and simplifieslist_lru_putback()to uselist_lru_from_memcg().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| mm/zswap.c | Switches zswap’s LRU operations to the new internal __list_lru_* helpers. |
| mm/list_lru.c | Renames the internal implementations to __list_lru_* and adds legacy 2-arg exported wrappers; adjusts list_lru_putback() lookup path. |
| include/linux/list_lru.h | Updates API declarations for __list_lru_* and re-adds 2-arg list_lru_* declarations. |
Comments suppressed due to low confidence (1)
include/linux/list_lru.h:135
- The kerneldoc comment header still says
list_lru_del, but the documented prototype here is now__list_lru_del(lru, item, nid, memcg). With the new legacy 2-arglist_lru_del()declaration, this will cause kerneldoc/parameter mismatch and mis-document the API. Rename the comment header to__list_lru_delto match the documented function signature.
bool __list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
struct mem_cgroup *memcg);
bool list_lru_del(struct list_lru *lru, struct list_head *item);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool __list_lru_add(struct list_lru *lru, struct list_head *item, int nid, | ||
| struct mem_cgroup *memcg); | ||
| bool list_lru_add(struct list_lru *lru, struct list_head *item); | ||
|
|
| bool __list_lru_add(struct list_lru *lru, struct list_head *item, int nid, | ||
| struct mem_cgroup *memcg); | ||
| bool list_lru_add(struct list_lru *lru, struct list_head *item); | ||
|
|
| * Return: true if the list was updated, false otherwise | ||
| */ | ||
| bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid, | ||
| bool __list_lru_del(struct list_lru *lru, struct list_head *item, int nid, |
deepin inclusion category: kapi Commit 7a40655 ("list_lru: allow explicit memcg and NUMA node selection") replaced the exported two-argument list_lru_add(lru, item) / list_lru_del(lru, item) with four-argument variants (adding @NiD and @memcg), and renamed the old behavior to list_lru_add_obj() / list_lru_del_obj(). All in-tree callers were updated to the _obj forms. This silently broke the EXPORT_SYMBOL_GPL KAPI list_lru_add(lru, item) / list_lru_del(lru, item): out-of-tree modules built against the pre-7a40655c4d3ae interface no longer compile (argument count mismatch). Restore KAPI compatibility with minimal changes: * rename the four-argument implementations to __list_lru_add() / __list_lru_del() (internal, not exported); * reintroduce the legacy two-argument list_lru_add(lru, item) / list_lru_del(lru, item) as EXPORT_SYMBOL_GPL wrappers forwarding to list_lru_add_obj() / list_lru_del_obj(), preserving the original semantics. The zswap shrinker introduced by 1d2290edafb38 ("zswap: make shrinking memcg-aware") calls the explicit four-argument form in zswap_lru_add()/zswap_lru_del() to pass @NiD and @memcg directly. Update those two call sites in mm/zswap.c to use __list_lru_add()/ __list_lru_del() so they keep targeting the explicit-selection implementation instead of the legacy two-argument wrapper. list_lru_putback() is unaffected. Assisted-by: atomcode:glm-5.2 Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
deepin inclusion category: bugfix After commit 2e60fe3 ("mm/list_lru: simplify reparenting and initial allocation"), memcg_reparent_list_lrus() erases the dying memcg's list_lru_memcg slot from the xarray, so list_lru_from_memcg_idx() can return NULL for a dying memcg. list_lru_add()/list_lru_del() handle this via list_lru_from_memcg(), which walks up to the parent. list_lru_putback() still used list_lru_from_memcg_idx() directly and dereferenced the result without a NULL check. Its only caller is the zswap shrinker: shrink_memcg_cb() -> zswap_lru_putback() on writeback failure, where the entry's memcg (obtained under rcu_read_lock()) may be concurrently offlined/reparented, leading to a NULL pointer dereference in list_add_tail(). Upstream never hit this because commit 5878303 removed the only caller and 3f798aa removed list_lru_putback() altogether; this branch keeps both, so switch list_lru_putback() to the same parent-fallback lookup used by list_lru_add()/list_lru_del(). Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
5bd67a0 to
13b0875
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Avenger-285714 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary by Sourcery
Restore legacy list_lru_add/list_lru_del kernel API while introducing internal memcg-aware helpers and updating zswap to use the new interfaces.
New Features:
Enhancements: