Re: [MAINTAINERS SUMMIT] The role of AI and LLMs in the kernel process

[Sasha Levin <sashal@kernel.org>]

On Tue, Aug 12, 2025 at 10:37:10AM -0400, Steven Rostedt wrote:
>One thing I look forward to with AI is that it will likely help me debug my
>own code. I suspect it would be really good at reading my code and say
>"Hey Stupid! You forgot to free this value over here". And things like that.
>In other words, I trust AI to debug my code more than I can debug its code.

I'm surprised that this isn't too common of a practice at this point.
Sure, AI will make mistakes, but I think that:

1. It's much better at reviewing rather than writing.
2. Mistakes aren't critical (you'd review it yourself anyway).
3. No legal issues.

As an example:

$ git ol 391dda1bd7c56de62b96126214f040fe8965561b
391dda1bd7c5 ("tracing: Use hashtable.h for event_hash")

$ git checkout 391dda1bd7c5^
HEAD is now at 7f81f27b1093 tracing: Fix use-after-free in print_graph_function_flags during tracer switching

$ git format-patch -1 391dda1bd7c5
0001-tracing-Use-hashtable.h-for-event_hash.patch

$ echo "I want to send @0001-tracing-Use-hashtable.h-for-event_hash.patch upstream. Carefully review the patch, identify any issues, and write a report." | claude -p
# Patch Review Report: tracing: Use hashtable.h for event_hash

## Summary
This patch converts the manually implemented hash table in `kernel/trace/trace_output.c` to use the generic hashtable API from `<linux/hashtable.h>`. The conversion is technically correct and simplifies the code, but there is one notable issue that should be addressed.

## Review Findings

### ✅ Positive Aspects

1. **Correct API Usage**: The patch properly uses the hashtable.h API:
    - `DEFINE_HASHTABLE()` for static initialization
    - `hash_add()` for insertion
    - `hash_del()` for deletion
    - `hash_for_each_possible()` for lookup

2. **Proper Constant Renaming**: The rename from `EVENT_HASHSIZE` to `EVENT_HASH_BITS` correctly reflects the semantic change (from size to bit count).

3. **Functionally Equivalent**: The new implementation maintains the same hash table size (2^7 = 128 entries) and hashing behavior.

4. **Code Simplification**: Removes manual bucket calculation logic, making the code cleaner and more maintainable.

### ❌ Issue Found

**Loss of `__read_mostly` Annotation**

The original implementation declares the hash table with `__read_mostly`:
```c
static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly;
```

The patch changes this to:
```c
static DEFINE_HASHTABLE(event_hash, EVENT_HASH_BITS);
```

The `DEFINE_HASHTABLE` macro does not preserve the `__read_mostly` annotation. This could impact performance as:
- The event hash table is primarily read during event lookups
- It's only written during event registration/unregistration (rare operations)
- Without `__read_mostly`, the hash table may not be optimally placed in cache-friendly memory sections

### Recommended Fix

The hashtable.h header provides `DEFINE_READ_MOSTLY_HASHTABLE` macro specifically for this use case. The patch should use:
```c
static DEFINE_READ_MOSTLY_HASHTABLE(event_hash, EVENT_HASH_BITS);
```

This would preserve the performance optimization of the original code.

## Verification Checklist

- [x] Patch applies cleanly
- [x] Hash table size remains unchanged (128 entries)
- [x] Hash function behavior is preserved
- [x] Locking semantics remain the same
- [x] All hash table operations correctly converted
- [ ] Performance annotations preserved (`__read_mostly`)

## Recommendation

**Hold for revision** - The patch should be updated to use `DEFINE_READ_MOSTLY_HASHTABLE` instead of `DEFINE_HASHTABLE` to maintain the performance optimization present in the original code.

-- 
Thanks,
Sasha