Claude review: dma-buf: system_heap: Optimize sg_table-to-pages conversion in vmap

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctness: Good**

The core transformation is equivalent. The original code does:
```c
for_each_sgtable_page(table, &piter, 0) {
    *tmp++ = sg_page_iter_page(&piter);
}
```
Where `sg_page_iter_page()` returns `sg_page(piter->sg) + piter->sg_pgoffset`.

The new code does:
```c
for_each_sgtable_sg(table, sg, i) {
    base_page = sg_page(sg);
    count = sg->length >> PAGE_SHIFT;
    for (j = 0; j < count; j++)
        *tmp++ = base_page + j;
}
```

These are functionally identical for the system heap's sg entries (offset=0, page-aligned lengths).

**Minor Issues:**

1. **Type of loop counter `i`**: The variable `i` is declared as `u32`, but `for_each_sgtable_sg` expands to `for_each_sg` which expects `int` for the counter. Looking at the macro:
   ```c
   #define for_each_sg(sglist, sg, nr, __i) \
       for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
   ```
   And `orig_nents` is `unsigned int`. Using `u32` will work but is inconsistent with typical kernel usage where `int` is used. Not a bug, but worth noting for style consistency.

2. **Type of `count`**: Using `u32` is fine since `sg->length` is `unsigned int` (32-bit), so `sg->length >> PAGE_SHIFT` always fits in a `u32`. No issue here.

3. **Commit message nit**: "we can populate the page array with a in an inner loop" has a stray "a" -- should be "with an inner loop" or "in an inner loop".

4. **WARN_ON semantic change is an improvement**: The original `WARN_ON(tmp - pages >= npages)` only guards against writing past the array. The new `WARN_ON(tmp - pages != npages)` after the loop detects both overflow *and* underflow (mismatch between `buffer->len` and the total sg lengths). This is strictly better.

5. **No bounds checking during the inner loop**: The original code had the `WARN_ON` inside the loop to catch overflow *before* writing. The new code removes this guard entirely and only checks *after* all writes. If there's ever a bug where the sg entries describe more pages than `npages`, the code will silently write past the `pages` array before the WARN_ON fires. In practice, this is fine for the system heap (the allocator creates the sg table from the same pages, so they must agree), and the performance benefit justifies removing the per-iteration check. But if reviewers feel strongly, an alternative would be `WARN_ON(tmp - pages > npages)` after the outer loop, before the vmap call -- which this patch effectively does with the `!=` check.

6. **Consider whether this pattern should use `sg_page_count()`**: There's no `sg_page_count()` helper in the kernel, so `sg->length >> PAGE_SHIFT` is the standard idiom. Fine as-is.

**Overall verdict**: The patch is correct and the optimization is well-motivated with good benchmarks. The only actionable feedback is the typo in the commit message. The rest is clean.

---
Generated by Claude Code Patch Reviewer