From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/xe: Avoid shrinker reclaim from kswapd under fragmentation
Date: Tue, 05 May 2026 10:00:01 +1000
Message-ID: <review-patch6-20260430191809.2142544-7-matthew.brost@intel.com>
In-Reply-To: <20260430191809.2142544-7-matthew.brost@intel.com>
References: <20260430191809.2142544-1-matthew.brost@intel.com> <20260430191809.2142544-7-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 6/6] drm/xe: Avoid shrinker reclaim from kswapd und=
er fragmentation`

```c
+	if (ttm_bo_shrink_kswap_maybe_fragmented(sc->nid, sc->order))
+		goto out;
```

**Placement concern:** This check is inserted at line 238 in `xe_shrinker_s=
can`, between the purgeable scan (phase 1) and the backup/writeback scan (p=
hase 2):

```c
	if (nr_scanned >=3D nr_to_scan || !can_backup)
		goto out;

+	if (ttm_bo_shrink_kswap_maybe_fragmented(sc->nid, sc->order))
+		goto out;

	/* If we didn't wake before, try to do it now if needed. */
	if (!runtime_pm)
		runtime_pm =3D xe_shrinker_runtime_pm_get(shrinker, true, 0, can_backup);
```

**This placement is well-chosen.** It allows purgeable BOs to still be free=
d (those are cheap to reclaim =E2=80=94 no writeback needed, just discarded=
). It only blocks the more expensive backup/writeback path where active GPU=
 working set would be evicted to swap/shmem. This is the right tradeoff: du=
ring fragmentation-driven kswapd, purge expendable objects but don't evict =
active working sets.

**However**, the `goto out` means `sc->nr_scanned` retains the value from t=
he purgeable scan, and the return is `nr_scanned ? freed : SHRINK_STOP`. If=
 the purgeable scan found nothing to scan (`nr_scanned =3D=3D 0`) and we hi=
t the fragmentation bail-out, we'll return `SHRINK_STOP`, which tells the M=
M "this shrinker has nothing to offer." That's actually the desired behavio=
r here =E2=80=94 if we have nothing purgeable and we're under fragmentation=
, we genuinely don't want kswapd to keep hammering this shrinker.

**No functional issues. Good integration.**

---

## Summary

The series addresses a real problem with a reasonable layered approach. **P=
atch 3 is the main concern** =E2=80=94 the change from `__GFP_DIRECT_RECLAI=
M` to `__GFP_RECLAIM` and the broadened condition from `order > beneficial_=
order` to `order !=3D beneficial_order` are both more aggressive than justi=
fied in the commit message, and the commit message itself is inconsistent w=
ith the implementation. The remaining patches are solid, with minor documen=
tation and style nits in patches 2 and 4.

---
Generated by Claude Code Patch Reviewer