> From 92dd9be5b5f3a225a7ca1befbe49d5f60e6ee908 Mon Sep 17 00:00:00 2001 > From: Jiri Pirko > Date: Thu, 15 Oct 2026 16:50:44 +0200 > Subject: [PATCH 11/20] ctlv: Prepare runtime introspection > > Add handlers that expose registered families, effective operations and > message schemas through the CTLV root family. > > The introspection object can be compile-checked here. CONFIG_CTLV remains This is not true at this point in the series. `introspection.c` dereferences `ctlvdev->dynamic_ops_count`, `->transition_lock`, `->state_generation` and ->disabled, and none of these exist in the `struct ctlv_device` that patch 10 creates (they are added by the dispatch patch). It also calls `ctlv_op_find()`, which is only defined in `core.c`. So the file neither compiles nor links until the last patch; "compile-checked" only holds if you do not actually compile it. Either move the device fields into the patch that first uses them (here) or drop the claim. Also note no `Documentation/ctlv` text or selftest accompanies the new op-schema-get/ops-dump/families-dump behaviour, and the fragment/resume semantics (entries split across fragments at member granularity, replayed ancestor nests) are only discoverable from this code. > unwired until device dispatch completes the module. > > Assisted-by: Cursor: GPT-5.6 Sol > Assisted-by: Cursor: Claude Opus 5.5 > Signed-off-by: Jiri Pirko > --- > drivers/ctlv/Makefile | 3 +- > drivers/ctlv/introspection.c | 512 +++++++++++++++++++++++++++++++++++ > 2 files changed, 514 insertions(+), 1 deletion(-) > create mode 100644 drivers/ctlv/introspection.c > > diff --git a/drivers/ctlv/Makefile b/drivers/ctlv/Makefile > index e631ccd68994..4d09fe26c1d0 100644 > --- a/drivers/ctlv/Makefile > +++ b/drivers/ctlv/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_CTLV) += ctlv.o > > -ctlv-y += blob.o blob_desc.o builder.o event.o message.o schema.o > +ctlv-y += blob.o blob_desc.o builder.o event.o introspection.o message.o \ > + schema.o > diff --git a/drivers/ctlv/introspection.c b/drivers/ctlv/introspection.c > new file mode 100644 > index 000000000000..7460dfa6d661 > --- /dev/null > +++ b/drivers/ctlv/introspection.c > @@ -0,0 +1,512 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES > + * > + * Root-family discovery queries and their fragmentation. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "device.h" > +#include "internal.h" > +#include "schema.h" > + > +/* A cursor packs the record and member where fragmentation resumes. */ The cursor is a client-supplied value (echoed back on continuation). Nothing validates it: a member > END or record >= count just yields an empty or "done" reply, which is harmless here, but the packing (48-bit record / 16-bit member) is an implicit ABI of the opaque cursor and is used by only two of the three dumps (op-schema-get uses a raw leaf ordinal). Three cursor encodings in one file; consider documenting them as opaque. > +#define CTLV_INTROSPECTION_MEMBER_SHIFT 48 > +#define CTLV_INTROSPECTION_RECORD_MASK GENMASK_ULL(CTLV_INTROSPECTION_MEMBER_SHIFT - 1, 0) > + > +struct ctlv_introspection_dump_position { > + u64 record; > + u32 member; > +}; > + > +static void ctlv_introspection_dump_resume(struct ctlv_introspection_dump_position *pos, u64 cursor) > +{ > + pos->record = cursor & CTLV_INTROSPECTION_RECORD_MASK; > + pos->member = cursor >> CTLV_INTROSPECTION_MEMBER_SHIFT; > +} > + > +static u64 ctlv_introspection_dump_cursor(const struct ctlv_introspection_dump_position *pos) > +{ > + return pos->record | (u64)pos->member << CTLV_INTROSPECTION_MEMBER_SHIFT; > +} > + > +/* The immutable root-first chain always has generation zero. */ > +static bool ctlv_introspection_family_record(struct ctlv_cursor *reply, struct ctlv_cursor *entry, Inconsistent with the ops dump: here the members are bare 0/1/2 magic numbers and two sequential 'if' blocks (a two-field state machine for a two-field entry), while the ops dump uses an enum and a loop. Splitting a two-leaf family entry across fragments at all is questionable generality; the minimum fragment size could simply guarantee one whole family entry fits, and the member part of the cursor would be unnecessary for this query. > + const struct ctlv_family *family, > + struct ctlv_introspection_dump_position *pos) > +{ > + const struct ctlv_family_schema *schema = family->schema; > + > + if (!ctlv_family_entry_start(reply, entry)) > + return false; > + if (pos->member == 0) { > + if (!ctlv_family_id_put(entry, schema->family_id)) > + return false; > + pos->member = 1; > + } > + if (pos->member == 1) { > + if (!ctlv_family_name_put(entry, schema->name)) > + return false; > + pos->member = 2; > + } > + ctlv_nest_end(entry); > + pos->member = 0; > + return true; > +} > + > +int ctlv_families_dump_query(void *priv, struct ctlv_op_ctx *ctx, const struct ctlv_cursor *request, > + struct ctlv_cursor *reply) > +{ > + const struct ctlv_family *family = ctx->family; > + u32 levels = ctlv_family_levels(family); > + struct ctlv_cursor entry; > + struct ctlv_introspection_dump_position pos; > + > + ctlv_introspection_dump_resume(&pos, ctlv_op_query_cursor(ctx)); > + for (; pos.record < levels; pos.record++) { > + const struct ctlv_family *ancestor = > + ctlv_family_ancestor(family, levels - 1 - pos.record); > + > + if (!ctlv_introspection_family_record(reply, &entry, ancestor, &pos)) > + return ctlv_op_query_more(ctx, 0, ctlv_introspection_dump_cursor(&pos)); > + } > + return ctlv_op_query_done(ctx, 0); > +} > + > +/* Snapshot published-op state and generation once per fragment. */ > +struct ctlv_op_snapshot { > + unsigned long *disabled; > + u64 generation; > +}; > + > +static int ctlv_introspection_snapshot_take(struct ctlv_device *ctlvdev, A `GFP_KERNEL` allocation per fragment just to copy a bitmap that is (for the published op set) at most a few words. Either use a fixed-size on-stack copy bounded by a device limit or read the bits with `test_bit_acquire()` like `ctlv_op_disabled()` does; the generation/bitmap pair only needs to be consistent per op, not per fragment, if the generation check on continuation already rejects changes. Also `snapshot->disabled` stays `NULL` when `dynamic_ops_count` == 0 and `op_disabled()` would then `test_bit(NULL)` if `state_index` were ever set; that is guaranteed only by `core.c`. > + struct ctlv_op_snapshot *snapshot) > +{ > + if (ctlvdev->dynamic_ops_count) { > + snapshot->disabled = bitmap_zalloc(ctlvdev->dynamic_ops_count, GFP_KERNEL); > + if (!snapshot->disabled) > + return -ENOMEM; > + } > + guard(spinlock) > + (&ctlvdev->transition_lock); > + snapshot->generation = ctlvdev->state_generation; > + if (snapshot->disabled) > + bitmap_copy(snapshot->disabled, ctlvdev->disabled, ctlvdev->dynamic_ops_count); > + return 0; > +} > + > +static bool ctlv_introspection_op_disabled(const struct ctlv_op_snapshot *snapshot, > + const struct ctlv_op_entry *op) > +{ > + if (op->state_index == CTLV_OP_STATE_NONE) > + return false; > + return test_bit(op->state_index, snapshot->disabled); > +} > + > +/* Report effective lengths, including descendant fills. */ > +enum ctlv_introspection_op_field { > + CTLV_INTROSPECTION_OP_FIELD_ID, > + CTLV_INTROSPECTION_OP_FIELD_TYPE, > + CTLV_INTROSPECTION_OP_FIELD_AVAILABILITY, > + CTLV_INTROSPECTION_OP_FIELD_DISABLED, > + CTLV_INTROSPECTION_OP_FIELD_REQUEST_MAX_LEN, > + CTLV_INTROSPECTION_OP_FIELD_ACTION_REPLY_MAX_LEN, > + CTLV_INTROSPECTION_OP_FIELD_QUERY_REPLY_MIN_LEN, > + CTLV_INTROSPECTION_OP_FIELD_QUERY_REPLY_SUGGESTED_LEN, > + CTLV_INTROSPECTION_OP_FIELD_EVENT_RECORD_MAX_LEN, > + CTLV_INTROSPECTION_OP_FIELD_FD_ACCESS, > + CTLV_INTROSPECTION_OP_FIELD_END, > +}; > + > +static bool ctlv_introspection_op_field_put(struct ctlv_cursor *entry, > + const struct ctlv_op_snapshot *snapshot, > + const struct ctlv_op_entry *op, u32 member) > +{ > + const struct ctlv_op_sizes *sizes = &op->sizes; > + > + switch (member) { > + case CTLV_INTROSPECTION_OP_FIELD_ID: > + return ctlv_op_id_put(entry, op->schema->op_id); > + case CTLV_INTROSPECTION_OP_FIELD_TYPE: > + return ctlv_op_type_put(entry, op->schema->type); > + case CTLV_INTROSPECTION_OP_FIELD_AVAILABILITY: > + return ctlv_op_availability_put(entry, op->schema->availability); > + case CTLV_INTROSPECTION_OP_FIELD_DISABLED: > + return ctlv_op_disabled_put(entry, ctlv_introspection_op_disabled(snapshot, op)); > + case CTLV_INTROSPECTION_OP_FIELD_REQUEST_MAX_LEN: > + return ctlv_op_request_max_len_put(entry, sizes->request_max_len); > + case CTLV_INTROSPECTION_OP_FIELD_ACTION_REPLY_MAX_LEN: > + /* Zero is a reply that acknowledges and no more. */ > + return ctlv_op_action_reply_max_len_put(entry, sizes->action_reply_max_len); > + case CTLV_INTROSPECTION_OP_FIELD_QUERY_REPLY_MIN_LEN: > + return ctlv_op_query_reply_min_len_put(entry, sizes->query_reply_min_len); > + case CTLV_INTROSPECTION_OP_FIELD_QUERY_REPLY_SUGGESTED_LEN: > + return ctlv_op_query_reply_suggested_len_put(entry, > + sizes->query_reply_suggested_len); > + case CTLV_INTROSPECTION_OP_FIELD_FD_ACCESS: > + return ctlv_op_fd_access_put(entry, op->schema->fd_access); > + default: > + return ctlv_op_event_record_max_len_put(entry, sizes->event_record_max_len); Reaching this via `default:` (anything that is not explicitly listed, including a future enum member added before `_END`) silently emits `event_record_max_len`. Make it an explicit case and let default WARN. Same in `op_field_applies()`, where 'default' means QUERY for two different members. > + } > +} > + > +static bool ctlv_introspection_op_field_applies(const struct ctlv_op_entry *op, u32 member) > +{ > + switch (member) { > + case CTLV_INTROSPECTION_OP_FIELD_ID: > + case CTLV_INTROSPECTION_OP_FIELD_TYPE: > + return true; > + case CTLV_INTROSPECTION_OP_FIELD_AVAILABILITY: > + case CTLV_INTROSPECTION_OP_FIELD_DISABLED: > + case CTLV_INTROSPECTION_OP_FIELD_REQUEST_MAX_LEN: > + case CTLV_INTROSPECTION_OP_FIELD_FD_ACCESS: > + return op->schema->type != CTLV_OP_TYPE_EVENT; > + case CTLV_INTROSPECTION_OP_FIELD_ACTION_REPLY_MAX_LEN: > + return op->schema->type == CTLV_OP_TYPE_ACTION; > + case CTLV_INTROSPECTION_OP_FIELD_EVENT_RECORD_MAX_LEN: > + return op->schema->type == CTLV_OP_TYPE_EVENT; > + default: > + return op->schema->type == CTLV_OP_TYPE_QUERY; > + } > +} > + > +static bool ctlv_introspection_op_record(struct ctlv_cursor *reply, struct ctlv_cursor *entry, > + const struct ctlv_op_snapshot *snapshot, > + const struct ctlv_op_entry *op, > + struct ctlv_introspection_dump_position *pos) > +{ > + if (!ctlv_op_entry_start(reply, entry)) > + return false; > + for (; pos->member < CTLV_INTROSPECTION_OP_FIELD_END; pos->member++) { > + if (!ctlv_introspection_op_field_applies(op, pos->member)) > + continue; > + if (!ctlv_introspection_op_field_put(entry, snapshot, op, pos->member)) > + return false; > + } > + ctlv_nest_end(entry); > + pos->member = 0; > + return true; > +} > + > +int ctlv_ops_dump_query(void *priv, struct ctlv_op_ctx *ctx, const struct ctlv_cursor *request, > + struct ctlv_cursor *reply) > +{ > + struct ctlv_device *ctlvdev = ctx->ctlvdev; > + struct ctlv_op_snapshot snapshot = {}; > + struct ctlv_cursor entry; > + struct ctlv_introspection_dump_position pos; > + int ret; > + > + ret = ctlv_introspection_snapshot_take(ctlvdev, &snapshot); > + if (ret) > + return ret; > + unsigned long *disabled __free(bitmap) = snapshot.disabled; Awkward double ownership: snapshot.disabled is allocated by `snapshot_take()` and a second variable, never used afterwards, exists only to run the cleanup. Make `snapshot_take()` return a `__free()`-able object or add a `snapshot_put()`. Mixed declaration after statements, and 'disabled' looks like dead code to a reader. > + > + /* The published index is immutable, so a cursor into it holds. */ > + ctlv_introspection_dump_resume(&pos, ctlv_op_query_cursor(ctx)); > + for (; pos.record < ctlvdev->ops_count; pos.record++) > + if (!ctlv_introspection_op_record(reply, &entry, &snapshot, > + &ctlvdev->ops[pos.record], &pos)) > + return ctlv_op_query_more(ctx, snapshot.generation, > + ctlv_introspection_dump_cursor(&pos)); > + return ctlv_op_query_done(ctx, snapshot.generation); > +} > + > +/* A leaf-count cursor permits a fragment boundary at any tree depth. */ Orphan comment: it is not attached to any declaration (blank line follows and then an unrelated struct). It also hides a cost: a continuation re-walks the whole schema from the start, skipping 'resume' leaves, so dumping a schema of L leaves in k fragments is O(k*L) and each fragment replays the ancestor nests (see `ctlv_introspection_open()`). Bounded by schema size, but worth noting. > + > +/* One nest of the description: the placement of one node of the schema. */ > +struct ctlv_introspection_frame { > + struct ctlv_cursor nest; > + u64 attr_id; > + bool open; > + /* Which node this stands for, so that a re-entry can be measured. */ > + const struct ctlv_schema_tree *tree; > + u32 node; > +}; > + > +/* The frames outlive the walk, a fragment's open nests having to stay alive. */ > +struct ctlv_introspection_walk { > + const struct ctlv_family *family; > + struct ctlv_cursor *root; > + struct ctlv_introspection_frame *frames; > + u64 ordinal; > + u64 resume; > + int error; > +}; > + > +/* One member of one node, which is every leaf this reply has. */ > +struct ctlv_introspection_field { > + u64 attr_id; > + u8 kind; > + u64 value; > +}; > + > +/* Three every node has, the widest bounds, a unit, a scale, a re-entry. */ > +#define CTLV_INTROSPECTION_MAX_FIELDS 10 The count is exactly the worst case (3 + 4 blob + 2 unit/scale + 1 reenters = 10), so the `WARN_ON_ONCE` in `ctlv_introspection_field()` is a dead branch unless a field is added. The fields[] array costs 10 * 24 bytes = 240 bytes on the stack, and is instantiated in every level of the `ctlv_introspection_node()` <-> `ctlv_introspection_children()` recursion (up to `CTLV_MAX_NEST_DEPTH` = 32 levels, plus the cursors), i.e. >8 KiB of kernel stack in one ioctl, on top of the ioctl path. Allocate it once in the walk (the frames are already heap allocated for this very reason) or emit directly without staging. > + > +struct ctlv_introspection_fields { > + struct ctlv_introspection_field fields[CTLV_INTROSPECTION_MAX_FIELDS]; > + u32 fields_count; > +}; > + > +static void ctlv_introspection_field(struct ctlv_introspection_fields *fields, u64 attr_id, u8 kind, > + u64 value) > +{ > + if (WARN_ON_ONCE(fields->fields_count == CTLV_INTROSPECTION_MAX_FIELDS)) > + return; > + fields->fields[fields->fields_count++] = (struct ctlv_introspection_field){ > + .attr_id = attr_id, > + .kind = kind, > + .value = value, > + }; > +} > + > +static void ctlv_introspection_uint(struct ctlv_introspection_fields *fields, u64 attr_id, > + u64 value) > +{ > + ctlv_introspection_field(fields, attr_id, CTLV_SCHEMA_UINT, value); > +} > + > +/* Emit only constraints for this node's payload kind. */ > +static void ctlv_introspection_node_fields(struct ctlv_introspection_fields *fields, > + const struct ctlv_node_schema *node, u32 reenters) > +{ > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_ATTR_ID, node->attr_id); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_KIND, node->kind); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_MAX_OCCURRENCES, node->max_occurrences); > + > + switch (node->kind) { > + case CTLV_SCHEMA_UINT: > + case CTLV_SCHEMA_SINT: > + case CTLV_SCHEMA_ENUM: > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_MIN, node->range.min); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_MAX, node->range.max); > + break; > + case CTLV_SCHEMA_FLAGS: > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_FLAGS_MASK, node->flags_mask); > + break; > + case CTLV_SCHEMA_STRING: > + case CTLV_SCHEMA_BINARY: > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_MIN_LENGTH, node->length.min_len); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_MAX_LENGTH, node->length.max_len); > + break; > + case CTLV_SCHEMA_BLOB: > + ctlv_introspection_field(fields, CTLV_ATTR_SCHEMA_BLOB_DIRECTION, CTLV_SCHEMA_ENUM, > + node->blob.direction); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_BLOB_BACKINGS, > + node->blob.backings); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_BLOB_MAX_LENGTH, > + node->blob.max_len); > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_BLOB_ALIGNMENT, > + node->blob.alignment); > + break; > + default: > + break; > + } > + > + if (node->unit != CTLV_UNIT_NONE) { > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_UNIT, node->unit); > + ctlv_introspection_field(fields, CTLV_ATTR_SCHEMA_SCALE, CTLV_SCHEMA_SINT, > + node->scale); > + } > + if (reenters) > + ctlv_introspection_uint(fields, CTLV_ATTR_SCHEMA_REENTERS, reenters); > +} > + > +/* A nest is opened by the first thing written inside it. */ > +static bool ctlv_introspection_open(struct ctlv_introspection_walk *walk, u32 depth) > +{ > + struct ctlv_introspection_frame *frame = &walk->frames[depth]; > + struct ctlv_cursor *up = walk->root; > + > + if (frame->open) > + return true; > + if (depth) { > + if (!ctlv_introspection_open(walk, depth - 1)) > + return false; > + up = &walk->frames[depth - 1].nest; > + } > + if (!ctlv_nest_start(up, &frame->nest, frame->attr_id, CTLV_SCHEMA_NEST)) > + return false; > + frame->open = true; > + return true; > +} > + > +/* Skipped leaves advance the ordinal; a failed write remains the resume point. */ > +static bool ctlv_introspection_put(struct ctlv_introspection_walk *walk, u32 depth, > + const struct ctlv_introspection_field *field) > +{ > + struct ctlv_cursor *nest = &walk->frames[depth].nest; > + bool ok; > + > + if (walk->ordinal < walk->resume) { > + walk->ordinal++; > + return true; > + } > + if (!ctlv_introspection_open(walk, depth)) > + return false; > + if (field->kind == CTLV_SCHEMA_SINT) > + ok = ctlv_put_s64(nest, field->attr_id, field->kind, field->value); > + else > + ok = ctlv_put_u64(nest, field->attr_id, field->kind, field->value); > + if (!ok) > + return false; > + walk->ordinal++; > + return true; > +} > + > +/* Re-entry targets an ancestor with the same child range. */ > +static u32 ctlv_introspection_reenters(struct ctlv_introspection_walk *walk, u32 depth, > + const struct ctlv_schema_tree *tree, > + const struct ctlv_node_schema *node) > +{ > + u32 levels; > + > + for (levels = 1; levels <= depth; levels++) { > + const struct ctlv_introspection_frame *up = &walk->frames[depth - levels]; > + > + if (up->tree == tree && > + tree->node_schemas[up->node].first_child == node->first_child) > + return levels; > + } > + /* A back edge to nothing is metadata this core cannot describe. */ If this cannot happen for a valid registered schema it is a registration-time check (done once), not something each query should WARN about and fail on. If it can happen, `WARN_ON_ONCE(1)` is the wrong tool for a data-dependent condition. > + WARN_ON_ONCE(1); > + walk->error = -EINVAL; > + return 0; > +} > + > +static bool ctlv_introspection_node(struct ctlv_introspection_walk *walk, u32 depth, > + const struct ctlv_schema_tree *tree, u32 index); > + > +/* Filled stub members inherit the stub placement's optionality. */ > +static bool ctlv_introspection_children(struct ctlv_introspection_walk *walk, u32 depth, > + const struct ctlv_schema_tree *tree, u32 index, > + bool force_optional) > +{ > + const struct ctlv_node_schema *node = &tree->node_schemas[index]; > + u32 i; > + > + if (WARN_ON_ONCE(depth + 1 >= CTLV_MAX_NEST_DEPTH)) { Fill recursion: `ctlv_introspection_children()` calls itself for stub fills with the same @depth, so this depth guard does not bound that recursion. Termination depends on fill schemas never nesting a stub that resolves back to the same fill, which is not checked here or (as far as I can see) at registration. At least WARN on a recursion counter. > + /* Deeper than a specification may declare, so unreachable. */ > + walk->error = -EINVAL; > + return false; > + } > + for (i = 0; i < node->children_count; i++) { > + u32 child = node->first_child + i; > + bool required = !force_optional && i < node->required_children_count; > + > + walk->frames[depth + 1] = (struct ctlv_introspection_frame){ > + .attr_id = required ? CTLV_ATTR_SCHEMA_ATTRS : > + CTLV_ATTR_SCHEMA_OPTIONAL_ATTRS, > + .tree = tree, > + .node = child, > + }; > + if (!ctlv_introspection_node(walk, depth + 1, tree, child)) > + return false; > + } > + for (i = 0; i < tree->stub_schemas_count; i++) { > + const struct ctlv_stub_schema *stub_schema = &tree->stub_schemas[i]; > + const struct ctlv_stub_fill_schema *fill_schema; > + > + if (stub_schema->node != index) > + continue; > + fill_schema = > + ctlv_schema_fill(walk->family, stub_schema->family_id, stub_schema->set_id); > + /* Nothing of this chain filled it, so it contributes none. */ > + if (!fill_schema) > + continue; > + /* A fill schema's root is the stub, its children the members. */ > + if (!ctlv_introspection_children(walk, depth, &fill_schema->members, 0, > + force_optional || stub_schema->optional)) > + return false; > + } > + return true; > +} > + > +/* One node, into the nest that stands for it. */ > +static bool ctlv_introspection_node(struct ctlv_introspection_walk *walk, u32 depth, > + const struct ctlv_schema_tree *tree, u32 index) > +{ > + const struct ctlv_node_schema *node = &tree->node_schemas[index]; > + struct ctlv_introspection_fields fields = {}; > + u32 reenters = 0; > + u32 i; > + > + if (node->reenters) { > + reenters = ctlv_introspection_reenters(walk, depth, tree, node); > + if (!reenters) > + return false; > + } > + ctlv_introspection_node_fields(&fields, node, reenters); > + for (i = 0; i < fields.fields_count; i++) > + if (!ctlv_introspection_put(walk, depth, &fields.fields[i])) > + return false; > + /* > + * A node that re-enters a level says so instead of listing that level's > + * members again, which is what makes an unbounded nesting describable. > + */ > + if (!node->reenters && !ctlv_introspection_children(walk, depth, tree, index, false)) > + return false; > + if (walk->frames[depth].open) > + ctlv_nest_end(&walk->frames[depth].nest); > + return true; > +} > + > +/* One of the two messages of an op, the root of it being a node like any. */ > +static bool ctlv_introspection_message(struct ctlv_introspection_walk *walk, u64 attr_id, > + const struct ctlv_schema_tree *tree) > +{ > + /* An op without such a message has nothing to describe. */ > + if (!tree->node_schemas) > + return true; > + walk->frames[0] = (struct ctlv_introspection_frame){ > + .attr_id = attr_id, > + .tree = tree, > + }; > + return ctlv_introspection_node(walk, 0, tree, 0); > +} > + > +int ctlv_op_schema_get_query(void *priv, struct ctlv_op_ctx *ctx, const struct ctlv_cursor *request, > + struct ctlv_cursor *reply) > +{ > + struct ctlv_introspection_walk walk = { > + .family = ctx->family, > + .root = reply, > + .resume = ctlv_op_query_cursor(ctx), > + }; > + const struct ctlv_op_entry *op; > + > + /* Describe published ops regardless of current dynamic state. */ > + op = ctlv_op_find(ctx->ctlvdev, ctlv_op_schema_get_op_id_get(request)); > + if (!op) > + return -EOPNOTSUPP; `-EOPNOTSUPP` for an unknown op id is misleading (the op is not unsupported, the argument is invalid/unknown). `-ENOENT` or `-EINVAL` with `ctlv_op_errorf("unknown op %#llx")` would give userspace a message like the subscription handlers in patch 10 do. The comment says published ops are described 'regardless of current dynamic state', but absent ops are compacted out of the array, so an optional op not provided is also `EOPNOTSUPP`; fine, but document it. > + > + /* > + * One frame per level the reply may nest, too many for the stack: a > + * description is one level deeper than the message it describes. > + */ > + struct ctlv_introspection_frame *frames __free(kfree) = > + kcalloc(CTLV_MAX_NEST_DEPTH, sizeof(*frames), GFP_KERNEL); patch 10 uses `kzalloc_obj()`; use the array variant of the new allocation API (`kzalloc_objs()`) here for consistency. Also the failure of a WARN'd error is returned with no `ctlv_op_error()` message, unlike the other paths. > + > + if (!frames) > + return -ENOMEM; > + walk.frames = frames; > + > + if (!ctlv_introspection_message(&walk, CTLV_ATTR_SCHEMA_REQUEST, &op->schema->request) || > + !ctlv_introspection_message(&walk, CTLV_ATTR_SCHEMA_REPLY, &op->schema->reply)) { > + if (walk.error) > + return walk.error; > + /* A schema is immutable, so there is nothing to be stale. */ Both `query_more` and `query_done` are passed generation 0 here. Fine, but note the same fragmenting cursor is therefore valid across device state changes; an op's description is static, OK. The cursor is the leaf ordinal and it must be non-zero and different from `ctx->frag`.cursor: if the first leaf does not fit (buffer smaller than `ctlv_op_schema_get`'s declared `query_reply_min_len`) the core WARNs and returns `-EINVAL` rather than an informative error; ensure the generator's min length covers one nest-start plus one leaf at the deepest nesting (32 levels of nest headers). > + return ctlv_op_query_more(ctx, 0, walk.ordinal); > + } > + return ctlv_op_query_done(ctx, 0); > +} > -- > 2.55.0 > Summary of the most serious issues: - The commit message claim that `introspection.c` can be compile-checked is false: it uses `ctlv_device` fields (`transition_lock`, `state_generation`, `dynamic_ops_count`, disabled) and `ctlv_op_find()` that only appear in the final dispatch patch. Series is not bisectable for this file. - Large per-level stack use (10-entry field array in every level of a 32-deep recursion), and unbounded fill-to-fill recursion not covered by the depth guard. - Needless `GFP_KERNEL` bitmap allocation per fragment plus awkward double ownership; default: cases that silently map unknown enum members to `event_record_max_len` / query members. - Inconsistent cursor encodings, `-EOPNOTSUPP` for an unknown op id, orphan comment, broken `guard()` formatting, and no documentation of the fragment resume semantics.