Claude review: drm/msm: Add PERFCNTR_CONFIG ioctl

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

Patch Review

This is the core ioctl implementation. Several observations:

**Bug -- Missing `-EAGAIN` for `O_NONBLOCK` reads:**

```c
static ssize_t
msm_perfcntrs_stream_read(struct file *file, char __user *buf,
			  size_t count, loff_t *ppos)
{
	...
	if (!(file->f_flags & O_NONBLOCK)) {
		ret = wait_event_interruptible(stream->poll_wq,
					       fifo_count(stream) > 0);
		if (ret)
			return ret;
	}

	guard(mutex)(&stream->read_lock);

	struct circ_buf *fifo = &stream->fifo;
	const char *fptr = &fifo->buf[fifo->tail];

	count = min_t(size_t, count, fifo_count_to_end(stream));
	if (copy_to_user(buf, fptr, count))
		return -EFAULT;
	...
	return count;
```

When `O_NONBLOCK` is set and the FIFO is empty, `fifo_count_to_end` returns 0, `count` becomes 0, and the function returns 0. A return of 0 from `read()` signals EOF to userspace. For a streaming fd, this should return `-EAGAIN`. Standard pattern would be:

```c
count = min_t(size_t, count, fifo_count_to_end(stream));
if (!count)
    return -EAGAIN;
```

This affects any userspace using `poll()` + `read()` with `O_NONBLOCK`, which is the expected usage pattern for a perfcntr stream fd.

**Locking and work cancellation design** is well thought-out. The comment about why `cancel_work()` (non-blocking) is used for `sel_work` inside `perfcntr_lock`, followed by `cancel_work_sync()` after dropping the lock in the release path, is excellent documentation of a subtle design decision.

**`fifo_count_to_end` uses `smp_load_acquire`** on the head:
```c
#define fifo_count_to_end(stream) \
	(CIRC_CNT_TO_END(smp_load_acquire(&(stream)->fifo.head), ...))
```

This pairs with `smp_store_release(&stream->fifo.head, head)` in `sample_worker`. Correct for the single-producer/single-consumer pattern.

**UABI design**: Using `DRM_IOW` to return an fd via the ioctl return value (rather than a struct field) avoids the fd-leak-on-copy_to_user-fault problem. The comment documenting this is helpful.

**Duplicate group detection**:
```c
if (nr_counters[idx])
    return UERR(EINVAL, dev, "groups[%d]: duplicate group", i);
```

This correctly catches duplicate groups, but note that if a group is requested with `nr_countables == 0`, it sets `nr_counters[idx] = 0`. A second occurrence of the same group with 0 countables would not be caught. This is arguably harmless (0 counters allocated either way) but slightly inconsistent.

---
Generated by Claude Code Patch Reviewer