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/page_alloc: check page->private upon page free Date: Tue, 24 Feb 2026 10:52:54 +1000 Message-ID: In-Reply-To: <20260223032641.1859381-12-ziy@nvidia.com> References: <20260223032641.1859381-1-ziy@nvidia.com> <20260223032641.1859381-12-ziy@nvidia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review > - page->private = 0; > + VM_WARN_ON_ONCE(page->private); This removes the safety-net zeroing of `page->private` for the head page and replaces it with only a warning. The loop body for sub-pages also only adds a warning: > (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP; > + VM_WARN_ON_ONCE((page + i)->private); Two concerns: First, `VM_WARN_ON_ONCE` only fires in debug kernels (when `CONFIG_DEBUG_VM` is set). In production kernels without this config, the `VM_WARN_ON_ONCE` compiles to nothing, meaning the old `page->private = 0` is simply deleted with no replacement. Any subsystem missed by patches 1-9 would silently leave stale data in private, potentially affecting the next page consumer. This is a correctness regression compared to the previous unconditional clearing. Second, this is `WARN_ON_ONCE`, so only the first violation will be reported. If there are multiple subsystems that still need fixing, only one will be visible per boot. The cover letter acknowledges the author "only tested part of" the subsystem fixes. Given that, removing the safety net in `__free_pages_prepare` seems premature. Would it be safer to keep the `page->private = 0` and add a `VM_WARN_ON_ONCE` check *before* the zeroing? That would catch violators while still maintaining the safety net: ```c VM_WARN_ON_ONCE(page->private); page->private = 0; ``` --- Generated by Claude Code Patch Reviewer