⚡ Bolt: [performance improvement] Use napi_alloc_skb - #7
Conversation
Replace dev_alloc_skb with napi_alloc_skb in the NAPI poll loop to utilize per-CPU NAPI cache. Co-authored-by: maxugly <64644401+maxugly@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe RX polling path now passes ChangesNAPI RX allocation
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the generic dev_alloc_skb with napi_alloc_skb inside the NAPI polling loop to utilize per-CPU cached allocations for better performance. The review feedback suggests adding a fallback to dev_alloc_skb in case napi is NULL to avoid potential kernel panics, and highlights that allocating memory while holding priv->lock is a performance anti-pattern that should be refactored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
|
|
||
| skb = dev_alloc_skb(hdr.len + 2); | ||
| skb = napi_alloc_skb(napi, hdr.len + 2); |
There was a problem hiding this comment.
Robustness & Performance Improvement
- Defensive Programming: If
napiisNULL(for instance, if this function is called from a non-NAPI context, testing, or fallback paths), passing it directly tonapi_alloc_skbcan lead to a kernel panic or undefined behavior depending on the kernel version. Adding a fallback todev_alloc_skbensures robustness. - Spinlock Holding Time: Calling
napi_alloc_skb(or any memory allocation) while holding the spinlockpriv->lockis a performance anti-pattern. Under memory pressure, the allocation can fall back to the page allocator, increasing lock holding time and causing severe lock contention with the TX path (nata_xmit).
Consider refactoring the RX path in a future iteration to perform the allocation outside of the spinlock (e.g., by peeking at the packet length locklessly first).
| skb = napi_alloc_skb(napi, hdr.len + 2); | |
| skb = napi ? napi_alloc_skb(napi, hdr.len + 2) : dev_alloc_skb(hdr.len + 2); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@module/nata_blk.c`:
- Line 240: Update section 6.3 of the RX dequeue specification to replace the
documented dev_alloc_skb behavior with napi_alloc_skb(napi, ...) and describe
the threaded NAPI context used by the dequeue path. Keep the documentation
aligned with the implementation around the skb allocation call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84993b8e-56e9-4d53-a653-ab0c21fb5dfd
📒 Files selected for processing (4)
.jules/bolt.mdmodule/nata.hmodule/nata_blk.cmodule/nata_net.c
| } | ||
|
|
||
| skb = dev_alloc_skb(hdr.len + 2); | ||
| skb = napi_alloc_skb(napi, hdr.len + 2); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the RX dequeue specification.
docs/specs/04-kernel-module.md still documents dev_alloc_skb; update section 6.3 to describe napi_alloc_skb(napi, ...) and the threaded NAPI context so the implementation and specification remain consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@module/nata_blk.c` at line 240, Update section 6.3 of the RX dequeue
specification to replace the documented dev_alloc_skb behavior with
napi_alloc_skb(napi, ...) and describe the threaded NAPI context used by the
dequeue path. Keep the documentation aligned with the implementation around the
skb allocation call.
1 participant
💡 What: Replace
dev_alloc_skbwithnapi_alloc_skbin the NAPIpollloop (nata_poll->sim_rx_dequeue). Also addedstruct napi_struct *napiparameter tosim_rx_dequeueto pass the context down.🎯 Why: Using
dev_alloc_skbuses the generic slab allocator, whereasnapi_alloc_skbuses a specialized per-CPU cache, avoiding lock contention and improving allocation performance during high-volume RX interrupts.📊 Impact: Expected reduction in latency and improved CPU utilization, leading to a bump in TCP/UDP throughput in high-load scenarios.
🔬 Measurement: Verify by benchmarking TCP throughput with
sudo ./scripts/nata-bench-once.sh. Ensure module loads, successfully handles traffic, and teardown doesn't result in leaks.PR created automatically by Jules for task 16511296223692522389 started by @maxugly
Summary by CodeRabbit
Performance
Documentation