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/gem-shmem: Return vm_fault_t from
 drm_gem_shmem_try_map_pmd()
Date: Wed, 11 Feb 2026 17:04:33 +1000
Message-ID: <review-patch3-20260209133241.238813-4-tzimmermann@suse.de>
In-Reply-To: <20260209133241.238813-4-tzimmermann@suse.de>
References: <20260209133241.238813-1-tzimmermann@suse.de> <20260209133241.238813-4-tzimmermann@suse.de>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-Mailer: Claude Code Patch Reviewer

Patch Review

**Review:**

Good API improvement suggested by Matthew Wilcox. Changes from boolean return to proper fault status:

```c
-static bool drm_gem_shmem_try_map_pmd(...)
+static vm_fault_t drm_gem_shmem_try_map_pmd(...)
```

**Before:**
```c
if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
    ret = VM_FAULT_NOPAGE;
    goto out;
}
```

**After:**
```c
ret = drm_gem_shmem_try_map_pmd(vmf, vmf->address, page);
if (ret == VM_FAULT_NOPAGE)
    goto out;
```

This is cleaner and allows the function to propagate error codes properly. The function now returns:
- `VM_FAULT_NOPAGE` if PMD mapping succeeded
- `0` if PMD mapping didn't happen (fall through to regular PFN mapping)
- Potentially error codes from `vmf_insert_pfn_pmd()`

**Issue - Incomplete error handling:**
The caller only checks for `VM_FAULT_NOPAGE`:
```c
ret = drm_gem_shmem_try_map_pmd(vmf, vmf->address, page);
if (ret == VM_FAULT_NOPAGE)
    goto out;
```

**What if `vmf_insert_pfn_pmd()` returns an error?** The code will fall through and attempt regular PFN insertion. Is this intentional fallback behavior or a bug?

Looking at the code flow:
1. If `ret == VM_FAULT_NOPAGE`: success, return
2. If `ret == 0`: PMD not attempted, fall through to regular PFN (correct)
3. If `ret == VM_FAULT_ERROR`: PMD failed, fall through to regular PFN (questionable)

**Verdict:** ⚠ Need clarification on error handling - should errors from PMD insertion fall through to regular PFN or be propagated?

---

---
Generated by Claude Code Patch Reviewer