From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: mm: Introduce zone_maybe_fragmented_in_shrinker() Date: Tue, 05 May 2026 10:00:00 +1000 Message-ID: In-Reply-To: <20260430191809.2142544-3-matthew.brost@intel.com> References: <20260430191809.2142544-1-matthew.brost@intel.com> <20260430191809.2142544-3-matthew.brost@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Subject:** `[PATCH v4 2/6] mm: Introduce zone_maybe_fragmented_in_shrinke= r()` Adds a static inline heuristic to `include/linux/vmstat.h`: ```c +static inline bool zone_maybe_fragmented_in_shrinker(struct zone *zone) +{ + return zone_page_state(zone, NR_FREE_PAGES) > + high_wmark_pages(zone) * 2; +} ``` **Concerns:** 1. **The 2x watermark threshold is arbitrary and undocumented.** The commit= message says "intentionally imprecise", which is fair, but the factor of 2= has no empirical justification in the commit message. It would be useful t= o explain why 2x specifically =E2=80=94 was this tuned experimentally? Diff= erent systems have very different watermark configurations. 2. **Overflow potential with `high_wmark_pages(zone) * 2`.** `high_wmark_pa= ges()` returns `unsigned long`. On 32-bit systems with large memory zones (= unlikely but theoretically possible), `* 2` could overflow. This is very un= likely to matter in practice but a `2UL *` would be safer. 3. **Placement in vmstat.h.** This is a policy heuristic, not a stat access= or. `vmstat.h` is about statistics, not reclaim policy decisions. MM mainta= iners may prefer this in a header more related to reclaim (e.g., `mm.h` or = a shrinker-specific header). Given the Cc list includes multiple MM maintai= ners, they may weigh in on placement. 4. **Name is good** =E2=80=94 the `_in_shrinker` suffix and `_maybe_` quali= fier set appropriate expectations. --- --- Generated by Claude Code Patch Reviewer