Re: [PATCH v10 1/3] workingset: refactor LRU refault to expose refault recency check
From: Matthew Wilcox <willy@infradead.org>
Date: 2023-02-19 12:05:55
Also in:
linux-mm, lkml
On Sat, Feb 18, 2023 at 11:33:16PM -0800, Nhat Pham wrote:
quoted hunk ↗ jump to hunk
+++ b/mm/workingset.c@@ -244,6 +244,30 @@ static void *lru_gen_eviction(struct folio *folio) return pack_shadow(mem_cgroup_id(memcg), pgdat, token, refs); } +/* + * Test if the folio is recently evicted. + * + * As a side effect, also populates the references with + * values unpacked from the shadow of the evicted folio. + */
I find this comment hard to understand. First it talks about "the folio", but it doesn't pass a folio. Then it talks about "the references", but I don't have any idea what those are either. I think what you mean is, * Test if the shadow entry is for a folio which was recently evicted. * Fills in @memcgid, @pgdat, @token and @workingset with values * extracted from the shadow entry.
+static bool lru_gen_test_recent(void *shadow, bool file, int *memcgid,
+ struct pglist_data **pgdat, unsigned long *token, bool *workingset)
+{
+ struct mem_cgroup *eviction_memcg;
+ struct lruvec *lruvec;
+ struct lru_gen_struct *lrugen;
+ unsigned long min_seq;
+
+ unpack_shadow(shadow, memcgid, pgdat, token, workingset);
+ eviction_memcg = mem_cgroup_from_id(*memcgid);
+
+ lruvec = mem_cgroup_lruvec(eviction_memcg, *pgdat);
+ lrugen = &lruvec->lrugen;
+
+ min_seq = READ_ONCE(lrugen->min_seq[file]);
+ return (*token >> LRU_REFS_WIDTH) == (min_seq & (EVICTION_MASK >> LRU_REFS_WIDTH));
+}[...]
+/* + * Test if the folio is recently evicted by checking if + * refault distance of shadow exceeds workingset size. * - * Calculates and evaluates the refault distance of the previously - * evicted folio in the context of the node and the memcg whose memory - * pressure caused the eviction. + * As a side effect, populate workingset with the value + * unpacked from shadow. */
1. Shouldn't this be kernel-doc? 2. Again, don't use the term "side effect" here. It's just one of the things that the function _does_.