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: lib/fonts: Manage font-data lifetime with font_data_get/_put()
Date: Tue, 10 Mar 2026 12:19:44 +1000
Message-ID: <review-patch9-20260309141723.137364-10-tzimmermann@suse.de>
In-Reply-To: <20260309141723.137364-10-tzimmermann@suse.de>
References: <20260309141723.137364-1-tzimmermann@suse.de> <20260309141723.137364-10-tzimmermann@suse.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is a significant refactoring. The reference counting logic is sound.

In `font_data_get()`:
```c
+	if (font_data_is_internal(fd))
+		return; /* never ref static data */
+
+	if (WARN_ON(!REFCOUNT(fd)))
+		return; /* should never be 0 */
+	++REFCOUNT(fd);
```
The `WARN_ON(!REFCOUNT(fd))` is unreachable: `font_data_is_internal()` already returns true when `REFCOUNT == 0`, so if we reach this line, `REFCOUNT != 0`. The WARN_ON could be removed, or `font_data_is_internal` could be changed to use a different check.

**Concurrency concern**: The `++REFCOUNT(fd)` and `--REFCOUNT(fd)` are non-atomic. This is presumably safe because all font operations are serialized under the console lock, but this assumption should be documented in the `font_data_get`/`font_data_put` kdoc comments.

In `font_data_free()`:
```c
static void font_data_free(font_data_t *fd)
{
	kfree(to_font_data_struct(fd));
}
```
Using `container_of` to get back to the `struct font_data` from `data[0]` is clean.

The removal of the `userfont` flag from `fbcon_display` is a nice simplification enabled by the internal/user-space distinction being handled transparently in get/put.

---
Generated by Claude Code Patch Reviewer