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:13:41 +1000 Message-ID: In-Reply-To: <20260430182335.2132382-3-matthew.brost@intel.com> References: <20260430182335.2132382-1-matthew.brost@intel.com> <20260430182335.2132382-3-matthew.brost@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Overall:** A simple heuristic helper placed in `vmstat.h`. The implementation is straightforward. ```c return zone_page_state(zone, NR_FREE_PAGES) > high_wmark_pages(zone) * 2; ``` **Concerns:** - **Naming:** The `_in_shrinker` suffix is unusual for a zone-level query. It documents the *intended caller* rather than the *semantic*. Something like `zone_has_free_above_high_wmark` would be more descriptive and wouldn't artificially limit reuse. That said, David Hildenbrand already suggested this name in v3 review, so this may be settled. - **Heuristic robustness:** The `2x high watermark` threshold is a magic number. On systems where the admin has tuned `min_free_kbytes` high, the watermarks could already be elevated, making this check less meaningful. The commit message acknowledges this is "intentionally imprecise," which is fine for a first pass, but adding a brief comment about the threshold choice directly in the code would help future readers. - **Inline in header:** Since this is an inline in a widely-included header (`vmstat.h`), it pulls in dependencies on `zone_page_state()` and `high_wmark_pages()`. These are already available in the vmstat.h context so this is fine technically, but it's a public API addition to core mm for what is currently only used by one GPU driver path. --- Generated by Claude Code Patch Reviewer