| 0/0 |
2026/04/05 16:17 |
flow |
assessment-kcsan |
23m
Results: map[Benign:true Confident:true Explanation:The KCSAN data race report identifies a race on the `pl->period` field within
the `fprop_reflect_period_percpu` function in `lib/flex_proportions.c`. This
function is part of the "flexible proportions" mechanism used by the Linux
kernel to track and age statistics for writeback throttling.
### Race Analysis
The race occurs between:
1. **Writer (Task 5216):** Updating `pl->period = period;` at line 135 while
holding the `pl->lock` spinlock.
2. **Reader (Task 6848):** Performing a lockless fast-path check `if
(pl->period == period)` at line 116.
This is a classic **double-checked locking** (or lockless fast-path) pattern.
The logic works as follows:
- The reader checks `pl->period` locklessly. If it matches the current global
period, the local statistics are already up to date, and the function returns
immediately.
- If the values do not match, the reader acquires `pl->lock` and performs a
second check: `if (pl->period >= period)`. This second check, performed under
the lock, ensures that only one thread performs the aging logic for a given
period transition.
### Severity: **BENIGN**
The race is considered **BENIGN** for the following reasons:
1. **Structural Tolerance:** The algorithm is designed to handle stale reads at
line 116. A stale read simply causes the thread to take the spinlock and
discover the update via the second check at line 120.
2. **Monotonicity:** The period counter is monotonically increasing. This
ensures that the checks `==` and `>=` are robust against typical race
conditions.
3. **Heuristic Nature:** The `flex_proportions` mechanism is used for writeback
heuristics (calculating dirty limits). Even if a race resulted in a slightly
delayed aging of a counter (e.g., if a reader saw a new period but not the aged
counter value due to memory reordering on non-x86 architectures), the impact
would be a negligible, transient inaccuracy in writeback throttling, not a
kernel crash or data corruption.
4. **Atomicity:** On the reported x86_64 architecture, 32-bit aligned accesses
to `unsigned int` are atomic, preventing torn reads. Even on architectures where
this might not be guaranteed, the double-check under the lock would resolve any
inconsistency.
### Recommended Annotations
While the race is benign, it is good practice to mark such intentional races to
satisfy static analysis tools and prevent aggressive compiler optimizations:
- Use `READ_ONCE(pl->period)` for the lockless read at line 116.
- Use `WRITE_ONCE(pl->period, period)` for the update at line 135.
- Alternatively, the lockless read can be wrapped in `data_race()`.]
|
| 1/1 |
2026/04/05 16:17 |
action |
kernel-checkouter |
1m
Results: map[KernelSrc:/app/workdir/cache/src/997bc7e8914d70427f9309efb2b13c37056003d5]
|
| 2/1 |
2026/04/05 16:18 |
action |
kernel-builder |
8m
Results: map[KernelObj:/app/workdir/cache/build/9cb71cf206f7143623aa56e9fa51b2a3ff83f214]
|
| 3/1 |
2026/04/05 16:27 |
action |
codesearch-prepare |
7m
Results: map[Index:codesearch-index]
|
| 4/1 |
2026/04/05 16:34 |
agent |
expert |
5m
Model:
Results: map[Benign:true Confident:true]
Instruction: # KCSAN Data Race Severity Analysis Protocol
You are an expert Linux kernel concurrency engineer analyzing a Kernel
Concurrency Sanitizer (KCSAN) data race report to determine if it is
**BENIGN** or **HARMFUL**.
## 1. CLASSIFICATIONS
### **BENIGN (Truly Benign)**
The logic is sound and structurally tolerant to compiler optimizations or
stale/torn reads.
- **Diagnostics/Stats:** Reads used only for '/proc', '/sys', counters, or
'pr_info'.
- **Heuristic Hints:** A "hint" flag where an old value only causes a
slightly delayed update or a sub-optimal but safe fast-path.
- **Single-Writer Flag Updates:** A single writer updating flags where the
concurrent read is a simple bitwise check (e.g., 'flags & MASK'). These are
historically tolerated, assuming neither "Fused Accesses" nor "Ordering
Violations" are relevant in this context.
- **Marked Reloads:** A load feeding into a 'cmpxchg()' loop or checked
against a later 'READ_ONCE()' reload.
- **Safe Overwrites:** Writing the same value already present.
### **HARMFUL (Logic Bug or Marking Required)**
The race causes incorrect behavior due to a synchronization failure or
because missing annotations allow the compiler to break the algorithm.
**Marking Required for Correctness:**
The algorithm is logically sound but requires annotations ('READ_ONCE()',
'WRITE_ONCE()', 'smp_load_acquire()', 'smp_store_release()', etc.) to be safe.
- **Fused Accesses:** The compiler might merge accesses or hoist a load out
of a loop, breaking polling/wait loops (livelocks).
- **Torn Accesses:** A large access (e.g., 64-bit on 32-bit arch) might be
split into multiple non-atomic accesses. Note that 'READ_ONCE()' does **not**
guarantee atomicity for 64-bit variables on 32-bit architectures.
- **Ordering Violations:** The race breaks a "happens-before" relationship
(requires primitives with implied or explicit memory barriers).
**Logic Bugs:**
A fundamental synchronization failure. Marking accesses will **not** fix it;
the logic itself must change.
- **Pointers/Lifecycle:** The racing variable is a pointer being dereferenced
or a refcount governing object lifecycle (Use-After-Free risk).
- **Control Flow:** The variable guards a critical section, memory allocation,
or hardware command.
- **Bitfields:** Concurrent writes to different bits in the same word.
Compilers often use non-atomic read-modify-write sequences, meaning a
write to 'bit_A' can "clobber" a concurrent write to 'bit_B'. However,
do not blindly assume all bitfield accesses are harmful; you must prove
that a concurrent write actually clobbers another in a way that breaks
logic.
- **Complex Structures:** Races on shared lists, trees, or hashmaps.
- **Lossy Updates:** Concurrent plain RMW operations (e.g., 'var++') on
non-diagnostic variables where every increment must be preserved.
- **State Machines:** Races allowing a state machine to bypass transitions
or enter an invalid state.
- **Adjacent Unsynchronized Operations:** Consider races happening at the
same time. For example, if both threads execute 'struct->has_elements = true;
list_add(node, &struct->list);', the race on 'has_elements' implies an
adjacent race on 'list_head', which is HARMFUL.
## 2. RESEARCH & ANALYSIS WORKFLOW
1. **Locate the Race:** Find the exact variables and functions in the stack
traces. **Use codesearch tools to read the actual source code and
confirm all assumptions.** Do not speculate about hypothetical compiler
behaviors or theoretical dangers (e.g., dismissing something as
"fundamentally unsafe") without tracing the actual data flow to a crash.
2. **Contextualize:** Identify held locks, RCU sections, or interrupt
contexts. Explain why the race is possible (e.g., "Thread A holds 'lock',
but Thread B is a lockless reader").
3. **Data Flow:** Follow the racing variable into its subsequent uses. If
the reader sees a stale or torn value, what branch is taken? Does it lead
to a pointer dereference or 'BUG_ON()'? **You must prove harm via this
data flow.**
4. **Failure Sequencing (if HARMFUL):** Construct a two-column execution trace
demonstrating the failure.
*Example format:*
CPU0 CPU1
function_a()
// lockless read
ptr = global_ptr
<PREEMPT>
function_b()
lock(&my_lock)
global_ptr = NULL
unlock(&my_lock)
kfree(ptr)
if (ptr)
*ptr = 1; // -> Use-After-Free!
## 3. OUTPUT FORMAT
- **Race Summary:** '[Function A]' vs '[Function B]' on '[Variable/Field]'.
- **Synchronization context:** List held locks and explain the lack of mutual
exclusion.
- **Final Classification:** **BENIGN** or **HARMFUL**.
**If BENIGN:**
- **Reasoning:** Briefly explain why the race is structurally tolerant to stale
or torn reads.
- **Recommended Annotations:** Suggest appropriate annotations (e.g.,
'READ_ONCE()', 'data_race()').
**If HARMFUL:**
- **Failure Reasoning:** Explain and provide a two-column interleaving showing
exactly how the race leads to a failure such as corruption or kernel crash.
- **Recommended Fix (ONLY if fix is trivial):** Suggest a structural fix (e.g.,
"Hold 'mapping->i_pages' lock", "Convert to 'atomic_t'") or required memory
ordering annotations (e.g., "Wrap in 'READ_ONCE()'", "Use
'smp_load_acquire()'").
Prefer calling several tools at the same time to save round-trips.
Use set-results tool to provide results of the analysis.
It must be called exactly once before the final reply.
Ignore results of this tool.
Prompt:
The data race report is:
==================================================================
BUG: KCSAN: data-race in fprop_reflect_period_percpu / fprop_reflect_period_percpu
write to 0xffff888100b159c8 of 4 bytes by task 5216 on cpu 1:
fprop_reflect_period_percpu+0x161/0x190 lib/flex_proportions.c:135
fprop_fraction_percpu+0x77/0x150 lib/flex_proportions.c:157
__wb_calc_thresh+0x89/0x2d0 mm/page-writeback.c:905
wb_bg_dirty_limits mm/page-writeback.c:2108 [inline]
domain_over_bg_thresh mm/page-writeback.c:2122 [inline]
wb_over_bg_thresh+0x222/0x530 mm/page-writeback.c:2143
wb_check_background_flush fs/fs-writeback.c:2301 [inline]
wb_do_writeback fs/fs-writeback.c:2399 [inline]
wb_workfn+0x690/0x970 fs/fs-writeback.c:2426
process_one_work kernel/workqueue.c:3276 [inline]
process_scheduled_works+0x513/0xa10 kernel/workqueue.c:3359
worker_thread+0x58a/0x780 kernel/workqueue.c:3440
kthread+0x22a/0x280 kernel/kthread.c:436
ret_from_fork+0x150/0x360 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
read to 0xffff888100b159c8 of 4 bytes by task 6848 on cpu 0:
fprop_reflect_period_percpu+0x31/0x190 lib/flex_proportions.c:116
__fprop_add_percpu lib/flex_proportions.c:143 [inline]
__fprop_add_percpu_max+0x57/0x190 lib/flex_proportions.c:201
wb_domain_writeout_add mm/page-writeback.c:554 [inline]
__wb_writeout_add+0x75/0x1b0 mm/page-writeback.c:578
__folio_end_writeback+0x14b/0x360 mm/page-writeback.c:2953
folio_end_writeback_no_dropbehind+0x6d/0x1d0 mm/filemap.c:1666
folio_end_writeback+0x1c/0x70 mm/filemap.c:1692
__block_write_full_folio+0x5f3/0x8f0 fs/buffer.c:1946
block_write_full_folio+0x2c1/0x2e0 fs/buffer.c:-1
blkdev_writepages+0x88/0x100 block/fops.c:486
do_writepages+0x1c6/0x310 mm/page-writeback.c:2554
filemap_writeback mm/filemap.c:387 [inline]
filemap_flush_range mm/filemap.c:436 [inline]
filemap_flush+0xee/0x130 mm/filemap.c:451
sync_blockdev_nowait+0x29/0x40 block/bdev.c:264
sync_filesystem+0xe2/0x190 fs/sync.c:60
generic_shutdown_super+0x4c/0x220 fs/super.c:625
kill_block_super+0x2a/0x70 fs/super.c:1725
ext4_kill_sb+0x42/0x80 fs/ext4/super.c:7484
deactivate_locked_super+0x75/0x1c0 fs/super.c:476
deactivate_super+0x97/0xa0 fs/super.c:509
cleanup_mnt+0x2bb/0x330 fs/namespace.c:1312
__cleanup_mnt+0x19/0x20 fs/namespace.c:1319
task_work_run+0x130/0x1a0 kernel/task_work.c:233
resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
__exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
exit_to_user_mode_loop+0x1f4/0x6f0 kernel/entry/common.c:98
__exit_to_user_mode_prepare include/linux/irq-entry-common.h:226 [inline]
syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:256 [inline]
syscall_exit_to_user_mode include/linux/entry-common.h:325 [inline]
do_syscall_64+0x249/0x370 arch/x86/entry/syscall_64.c:100
entry_SYSCALL_64_after_hwframe+0x77/0x7f
value changed: 0x0000002b -> 0x0000002c
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 UID: 0 PID: 6848 Comm: syz-executor Tainted: G W syzkaller #0 PREEMPT(full)
Tainted: [W]=WARN
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/18/2026
==================================================================
EXT4-fs (loop7): unmounting filesystem 76b65be2-f6da-4727-8c75-0525a5b65a09.
Reply: The KCSAN data race report identifies a race on the `pl->period` field within the `fprop_reflect_period_percpu` function in `lib/flex_proportions.c`. This function is part of the "flexible proportions" mechanism used by the Linux kernel to track and age statistics for writeback throttling.
### Race Analysis
The race occurs between:
1. **Writer (Task 5216):** Updating `pl->period = period;` at line 135 while holding the `pl->lock` spinlock.
2. **Reader (Task 6848):** Performing a lockless fast-path check `if (pl->period == period)` at line 116.
This is a classic **double-checked locking** (or lockless fast-path) pattern. The logic works as follows:
- The reader checks `pl->period` locklessly. If it matches the current global period, the local statistics are already up to date, and the function returns immediately.
- If the values do not match, the reader acquires `pl->lock` and performs a second check: `if (pl->period >= period)`. This second check, performed under the lock, ensures that only one thread performs the aging logic for a given period transition.
### Severity: **BENIGN**
The race is considered **BENIGN** for the following reasons:
1. **Structural Tolerance:** The algorithm is designed to handle stale reads at line 116. A stale read simply causes the thread to take the spinlock and discover the update via the second check at line 120.
2. **Monotonicity:** The period counter is monotonically increasing. This ensures that the checks `==` and `>=` are robust against typical race conditions.
3. **Heuristic Nature:** The `flex_proportions` mechanism is used for writeback heuristics (calculating dirty limits). Even if a race resulted in a slightly delayed aging of a counter (e.g., if a reader saw a new period but not the aged counter value due to memory reordering on non-x86 architectures), the impact would be a negligible, transient inaccuracy in writeback throttling, not a kernel crash or data corruption.
4. **Atomicity:** On the reported x86_64 architecture, 32-bit aligned accesses to `unsigned int` are atomic, preventing torn reads. Even on architectures where this might not be guaranteed, the double-check under the lock would resolve any inconsistency.
### Recommended Annotations
While the race is benign, it is good practice to mark such intentional races to satisfy static analysis tools and prevent aggressive compiler optimizations:
- Use `READ_ONCE(pl->period)` for the lockless read at line 116.
- Use `WRITE_ONCE(pl->period, period)` for the update at line 135.
- Alternatively, the lockless read can be wrapped in `data_race()`.
|
| 5/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 5593
output: 47
thoughts: 86
Thoughts: **Analyzing a Data Race in `fprop_reflect_period_percpu`**
Okay, so the report just flagged a data race, and it looks like it's happening in `fprop_reflect_period_percpu`. Specifically, it's a conflict between a write and a read operation on a 4-byte variable. The address given is `0xffff888100b159c8`. I need to drill down on this. The value flipped from `0x2b` to `0x2c`, a small change, but significant if timing-sensitive.
My immediate next step is to get into `lib/flex_proportions.c` and figure out what's going on. This is where the race condition is supposedly manifesting. I'll need to identify the exact code segments involved in both the read and write operations on that memory location. I need to understand the logic flow, the locking mechanisms (or lack thereof!), and the potential for concurrent access. This is classic debugging - identify the conflicting lines and reason about their behavior.
|
| 6/2 |
2026/04/05 16:34 |
tool |
codesearch-dir-index |
0m
Args:
Results: map[Files:[alloc_tag.c argv_split.c ashldi3.c ashrdi3.c asn1_decoder.c asn1_encoder.c assoc_array.c atomic64.c atomic64_test.c audit.c base64.c bcd.c bch.c bitmap-str.c bitmap.c bitrev.c bootconfig-data.S bootconfig.c bsearch.c btree.c bucket_locks.c bug.c buildid.c bust_spinlocks.c cache_maint.c check_signature.c checksum.c closure.c clz_ctz.c clz_tab.c cmdline.c cmpdi2.c cmpxchg-emu.c codetag.c compat_audit.c cpu_rmap.c cpumask.c ctype.c debug_info.c debug_locks.c debugobjects.c dec_and_lock.c decompress.c decompress_bunzip2.c decompress_inflate.c decompress_unlz4.c decompress_unlzma.c decompress_unlzo.c decompress_unxz.c decompress_unzstd.c devmem_is_allowed.c devres.c dhry.h dhry_1.c dhry_2.c dhry_run.c digsig.c dump_stack.c dynamic_debug.c dynamic_queue_limits.c earlycpio.c errname.c error-inject.c errseq.c extable.c fault-inject-usercopy.c fault-inject.c fdt.c fdt_addresses.c fdt_empty_tree.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c fdt_wip.c find_bit.c find_bit_benchmark.c find_bit_benchmark_rust.rs flex_proportions.c fw_table.c genalloc.c generic-radix-tree.c glob.c group_cpus.c hexdump.c hweight.c idr.c inflate.c interval_tree.c interval_tree_test.c iomap.c iomap_copy.c iomem_copy.c iommu-helper.c iov_iter.c irq_poll.c irq_regs.c is_single_threaded.c kasprintf.c kfifo.c klist.c kobject.c kobject_uevent.c kstrtox.c kstrtox.h linear_ranges.c list_debug.c list_sort.c llist.c locking-selftest-hardirq.h locking-selftest-mutex.h locking-selftest-rlock-hardirq.h locking-selftest-rlock-softirq.h locking-selftest-rlock.h locking-selftest-rsem.h locking-selftest-rtmutex.h locking-selftest-softirq.h locking-selftest-spin-hardirq.h locking-selftest-spin-softirq.h locking-selftest-spin.h locking-selftest-wlock-hardirq.h locking-selftest-wlock-softirq.h locking-selftest-wlock.h locking-selftest-wsem.h locking-selftest.c lockref.c logic_iomem.c logic_pio.c lru_cache.c lshrdi3.c lwq.c maple_tree.c memcat_p.c memory-notifier-error-inject.c memregion.c memweight.c min_heap.c muldi3.c net_utils.c netdev-notifier-error-inject.c nlattr.c nmi_backtrace.c notifier-error-inject.c notifier-error-inject.h objagg.c objpool.c of-reconfig-notifier-error-inject.c oid_registry.c oid_registry_data.c once.c packing.c packing_test.c parman.c parser.c percpu-refcount.c percpu_counter.c percpu_test.c plist.c pm-notifier-error-inject.c polynomial.c radix-tree.c radix-tree.h random32.c ratelimit.c rbtree.c rbtree_test.c rcuref.c ref_tracker.c refcount.c rhashtable.c sbitmap.c scatterlist.c seq_buf.c sg_pool.c sg_split.c siphash.c smp_processor_id.c sort.c stackdepot.c stmp_device.c string.c string_helpers.c strncpy_from_user.c strnlen_user.c sys_info.c syscall.c test-kstrtox.c test_bitmap.c test_bitops.c test_bpf.c test_context-analysis.c test_debug_virtual.c test_dynamic_debug.c test_firmware.c test_fpu.h test_fpu_glue.c test_fpu_impl.c test_free_pages.c test_hexdump.c test_hmm.c test_hmm_uapi.h test_ida.c test_kho.c test_kmod.c test_lockup.c test_maple_tree.c test_memcat_p.c test_meminit.c test_module.c test_objagg.c test_objpool.c test_parman.c test_ref_tracker.c test_rhashtable.c test_static_key_base.c test_static_keys.c test_sysctl.c test_ubsan.c test_vmalloc.c test_xarray.c textsearch.c timerqueue.c trace_readwrite.c ts_bm.c ts_fsm.c ts_kmp.c ubsan.c ubsan.h ucmpdi2.c ucs2_string.c union_find.c usercopy.c uuid.c vsprintf.c win_minmax.c xarray.c xxhash.c] Subdirs:[842 crc crypto dim fonts kunit lz4 lzo math pldmfw raid6 reed_solomon test_fortify tests vdso xz zlib_deflate zlib_dfltcc zlib_inflate zstd]]
|
| 7/2 |
2026/04/05 16:34 |
tool |
codesearch-file-index |
0m
Args: map[SourceFile:lib/flex_proportions.c]
Results: map[Entities:[map[Kind:function Name:__fprop_add_percpu] map[Kind:function Name:__fprop_add_percpu_max] map[Kind:function Name:fprop_fraction_percpu] map[Kind:function Name:fprop_global_destroy] map[Kind:function Name:fprop_global_init] map[Kind:function Name:fprop_local_destroy_percpu] map[Kind:function Name:fprop_local_init_percpu] map[Kind:function Name:fprop_new_period] map[Kind:function Name:fprop_reflect_period_percpu]]]
|
| 8/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 8343
output: 46
thoughts: 19
Thoughts: Here's a breakdown of my thought process:
**Diving into fprop_reflect_period_percpu**
Okay, so I'm presented with `fprop_reflect_period_percpu`. My initial thought is, "Right, another performance-related variable, likely tied to frequency propagation or some form of reflection within a CPU context." The "percpu" immediately clues me in to the fact that this is something that's tracked on a per-CPU basis, which is key for understanding its implications in a multi-core environment. I immediately want to dig into the source code to understand its purpose, what's it's intended use and to grasp the algorithms or data structures it's interacting with, this is critical in order to understand performance bottlenecks or potential race conditions. I need to understand its context and whether it's related to clock frequency scaling, power management, or some other aspect of CPU behavior. I need to be able to understand the control flow. What functions read and write to this variable? When is it updated, and under what conditions? Is there a spinlock protecting this variable? What is its size and data type? Does this variable provide a measure of CPU activity? I really need to get in there and see how this variable is used.
|
| 9/2 |
2026/04/05 16:34 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_reflect_period_percpu]
Results: map[SourceCode: 109: static void fprop_reflect_period_percpu(struct fprop_global *p,
110: struct fprop_local_percpu *pl)
111: {
112: unsigned int period = p->period;
113: unsigned long flags;
114:
115: /* Fast path - period didn't change */
116: if (pl->period == period)
117: return;
118: raw_spin_lock_irqsave(&pl->lock, flags);
119: /* Someone updated pl->period while we were spinning? */
120: if (pl->period >= period) {
121: raw_spin_unlock_irqrestore(&pl->lock, flags);
122: return;
123: }
124: /* Aging zeroed our fraction? */
125: if (period - pl->period < BITS_PER_LONG) {
126: s64 val = percpu_counter_read(&pl->events);
127:
128: if (val < (nr_cpu_ids * PROP_BATCH))
129: val = percpu_counter_sum(&pl->events);
130:
131: percpu_counter_add_batch(&pl->events,
132: -val + (val >> (period-pl->period)), PROP_BATCH);
133: } else
134: percpu_counter_set(&pl->events, 0);
135: pl->period = period;
136: raw_spin_unlock_irqrestore(&pl->lock, flags);
137: }
SourceFile:lib/flex_proportions.c]
|
| 10/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 9165
output: 44
thoughts: 752
Thoughts: **Analyzing a Potential Race Condition**
Okay, so we have a race condition being reported involving a `pl->period` variable. Task 5216, the writer, is updating it at line 135 in `lib/flex_proportions.c`, and task 6848, the reader, is checking it at line 116. The report tells us the memory address being accessed is `0xffff888100b159c8` in both cases, which strongly suggests it's the same variable.
The write operation happens within a critical section protected by `raw_spin_lock_irqsave(&pl->lock, flags)`. However, the read at line 116 is lockless, which is the immediate red flag. It looks like a classic double-checked locking pattern is in play, or at least a lockless fast path. The writer updates `pl->period` inside the lock, then the reader initially checks `pl->period` without a lock. If it *doesn't* match the expected `period`, it enters the critical section and re-checks it (line 120).
The key here is that `pl->period` is just a plain `unsigned int`. My initial concern is whether the compiler optimizations could break this assumption. However, looking at the access, its 4 bytes so it is likely atomic on x86_64, which is indicated by the stack traces referencing architecture-specific assembly.
Let's trace a scenario. The report says the value changed from 43 (0x2b) to 44 (0x2c). If the reader at line 116 reads the *old* value, 43, while the writer is writing 44, and the `period` the reader is expecting is 44, the reader will take the slow path and grab the lock. Once inside the lock, the reader will see the updated value of 44, which is correct. On the other hand, if the reader reads 44 before the writer takes the lock, it will return immediately, also correct.
The most likely issue I need to worry about is a torn read. Since it's a 4-byte `unsigned int` on x86_64, which uses little-endian byte order, aligned 4-byte accesses *should* be atomic. So, tearing *shouldn't* be an issue.
The function `fprop_reflect_period_percpu` seems to be involved in aging proportions. I will examine the `fprop_global` and `fprop_local_percpu` structures to see if there is any other potential harm, but at this point, it looks like this code is correctly implemented.
|
| 11/2 |
2026/04/05 16:34 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_local_percpu]
Results: map[SourceCode: 41: /*
42: * ---- PERCPU ----
43: */
44: struct fprop_local_percpu {
45: /* the local events counter */
46: struct percpu_counter events;
47: /* Period in which we last updated events */
48: unsigned int period;
49: raw_spinlock_t lock; /* Protect period and numerator */
50: };
SourceFile:include/linux/flex_proportions.h]
|
| 12/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 10701
output: 41
thoughts: 9
Thoughts: Okay, here's my interpretation of that thought process, tailored for an expert audience, and written in the first person:
**My Deep Dive into `fprop_global`**
Alright, let's break down this `fprop_global` structure. Immediately, I'm thinking about the implications for global forward propagation in whatever neural network framework we're dealing with. The "global" aspect suggests we're probably not talking about layer-wise operations but something that spans the entire network, or at least a significant portion of it. I'm imagining this could be used for things like:
* **Shared parameters:** Does this structure manage any global parameters shared across multiple layers or modules? Think things like batch normalization statistics, global attention mechanisms, or even regularization terms. This could represent a central repository for anything that needs network-wide coordination.
* **Context for Communication:** Is this structure used to pass context between different parts of the network during the forward pass? Perhaps information about the input's trajectory, intermediate activations, or even debugging information. I am considering the implications of this.
* **Computational Graph Optimization:** I wonder if this structure plays a role in optimizing the computational graph. Could it hold information about intermediate values or dependencies that can be used to fuse operations or prune unnecessary computations? It could be a way to track the network's internal state efficiently.
* **Distributed Training Support:** Given the "global" nature, I'm curious if this structure facilitates distributed training. Could it manage synchronization points, communication buffers, or even model partitioning information? Handling distributed scenarios can add some complexity.
* **Error Handling:** It would be good to have a system for logging any errors or warnings during global propagation, and so I am considering whether this would serve that purpose.
I'm starting to think about what data types are used within the structure. What's the size and scope of memory this requires? Are we dealing with pointers, shared memory, or some form of data serialization? This can influence performance dramatically. Finally, how is this structure initialized and updated? Are there any thread-safety considerations? The devil is in the details, so I'll need to dig deeper into the source code to get the full picture.
|
| 13/2 |
2026/04/05 16:34 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_global]
Results: map[SourceCode: 25: /*
26: * ---- Global proportion definitions ----
27: */
28: struct fprop_global {
29: /* Number of events in the current period */
30: struct percpu_counter events;
31: /* Current period */
32: unsigned int period;
33: /* Synchronization with period transitions */
34: seqcount_t sequence;
35: };
SourceFile:include/linux/flex_proportions.h]
|
| 14/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 11368
output: 44
thoughts: 103
Thoughts: **Analyzing the Data Flow - Period Update**
Okay, so the race condition is centered around `pl->period`. The writer is updating it on line 135: `pl->period = period;`. My attention is immediately drawn to line 116: `if (pl->period == period)`. That's where a check is happening and where the read is occurring, and thus the point that might encounter a race condition.
I'm currently within `fprop_reflect_period_percpu`, which is called either by `fprop_fraction_percpu` or directly from `__fprop_add_percpu`. Let's dig into `fprop_fraction_percpu` to understand the context of this read/write pair and the potential for concurrent access. I need to understand how these functions are being called and what data they're manipulating to properly evaluate this potential race. I'll focus on the data dependencies and see if there are any obvious locking mechanisms in place.
|
| 15/2 |
2026/04/05 16:34 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_fraction_percpu]
Results: map[SourceCode: 148: void fprop_fraction_percpu(struct fprop_global *p,
149: struct fprop_local_percpu *pl,
150: unsigned long *numerator, unsigned long *denominator)
151: {
152: unsigned int seq;
153: s64 num, den;
154:
155: do {
156: seq = read_seqcount_begin(&p->sequence);
157: fprop_reflect_period_percpu(p, pl);
158: num = percpu_counter_read_positive(&pl->events);
159: den = percpu_counter_read_positive(&p->events);
160: } while (read_seqcount_retry(&p->sequence, seq));
161:
162: /*
163: * Make fraction <= 1 and denominator > 0 even in presence of percpu
164: * counter errors
165: */
166: if (den <= num) {
167: if (num)
168: den = num;
169: else
170: den = 1;
171: }
172: *denominator = den;
173: *numerator = num;
174: }
SourceFile:lib/flex_proportions.c]
|
| 16/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 12129
output: 45
thoughts: 130
Thoughts: **Tracing the Writeback Flow**
Okay, so I'm currently looking at a writeback issue, and the call stack is a bit tangled. Let me break down where I am. I'm focusing on `fprop_fraction_percpu`, which is triggered from `__wb_calc_thresh`. Right now, I'm working my way through `fprop_reflect_period_percpu`. I see the writer is at line 135 of `fprop_reflect_period_percpu`, which got called from `fprop_fraction_percpu` (line 157). That's the current context for writing something. The reader is at line 116 in `fprop_reflect_period_percpu`, which is also reached through `__fprop_add_percpu` (specifically via `__fprop_add_percpu_max`). To get a better grasp of the situation, I should probably take a closer look at `__fprop_add_percpu`. That's where I need to start digging in.
|
| 17/2 |
2026/04/05 16:34 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:__fprop_add_percpu]
Results: map[SourceCode: 139: /* Event of type pl happened */
140: void __fprop_add_percpu(struct fprop_global *p, struct fprop_local_percpu *pl,
141: long nr)
142: {
143: fprop_reflect_period_percpu(p, pl);
144: percpu_counter_add_batch(&pl->events, nr, PROP_BATCH);
145: percpu_counter_add(&p->events, nr);
146: }
SourceFile:lib/flex_proportions.c]
|
| 18/2 |
2026/04/05 16:34 |
llm |
expert |
0m
Model:
Tokens:
input: 12707
output: 45
thoughts: 1257
Thoughts: **Analyzing a Potential Data Race in `fprop_reflect_period_percpu`**
Okay, so here's what I'm seeing. We have a classic read-write scenario on the `pl->period` variable within `fprop_reflect_period_percpu`. Task 5216 on CPU 1 is the writer, and Task 6848 on CPU 0 is the reader. Both are operating within the `lib/flex_proportions.c` file. The writer's execution path is `fprop_fraction_percpu -> fprop_reflect_period_percpu`, while the reader is on `__fprop_add_percpu -> fprop_reflect_period_percpu`.
The core issue is that `pl->period` is being updated (written) by the writer at lib/flex_proportions.c:135, and read by the reader at lib/flex_proportions.c:116. This happens within a `read_seqcount_begin/retry` loop for the writer. This is a crucial detail because of how that loop interacts with the locking mechanism.
My initial thought is this looks like a double-checked locking optimization. The reader first performs a lockless read, and only if the read value is stale, it will try to acquire a lock. If it takes the lock, it re-reads the value, and potentially then does some work. The writer holds the lock while updating the value. Sounds good, right?
But immediately, my spidey-sense is tingling. Where are the memory barriers? This is where things get tricky. We have `pl->period` which is the focus of this lockless read. The `WRITE_ONCE` and `READ_ONCE` primitives are noticeably absent on the access to `pl->period`. So if the compiler reorders the access, we could have a problem.
The compiler reordering a load after the lock check is fine as it takes the lock. Reordering inside the lock is also fine as the locking provides the barrier.
However, the real concern is the write reordering. The write to `pl->period` at line 135 is inside a `raw_spin_lock_irqsave/unlock_irqrestore` block. The `raw_spin_unlock_irqrestore` provides a release barrier, so the write to `pl->period` cannot be reordered *after* the unlock, which is good. But can writes *before* it, be reordered? The lock should provide the needed synchronization there.
Then there is the reader. It does a read without a barrier. If the compiler moves operations *before* line 116 into the lockless read area? Hmm...
But it gets worse. I see that `fprop_reflect_period_percpu` *also* updates `pl->events`. The writer modifies `pl->events` (at line 131 or 134) and *then* updates `pl->period` at line 135. The reader checks `pl->period` at 116.
If the reader sees the *new* value of `pl->period` at line 116 but not the updated `pl->events`, *that* could be a problem! The writer holds the lock while modifying *both* the `pl->events` and `pl->period`. The reader's lockless check assumes data consistency. `pl->events` is a `percpu_counter`, which, I now realize, is *very* important.
The writer is updating `pl->events` *based* on its previous value. A batch is applied to the counter. So the real question now is, is the update of `percpu_counter` atomic? If the reader sees the new `pl->period` but the `percpu_counter_add_batch` hasn't completed or its effects are not visible, we could be using stale values of `pl->events`. I need to dig into `percpu_counter_add_batch` to be sure. It's likely using locks or atomics, but I need to confirm. And, honestly, *even if* `percpu_counter` uses atomics internally, the lack of `READ_ONCE` *still* worries me. This lockless read is still concerning. The lack of `READ_ONCE` on line 116. This might actually be a dangerous optimization.
|
| 19/2 |
2026/04/05 16:35 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:percpu_counter_add_batch]
Results: map[SourceCode: 88: /*
89: * Safety against interrupts is achieved in 2 ways:
90: * 1. the fast path uses local cmpxchg (note: no lock prefix)
91: * 2. the slow path operates with interrupts disabled
92: */
93: void percpu_counter_add_batch(struct percpu_counter *fbc, s64 amount, s32 batch)
94: {
95: s64 count;
96: unsigned long flags;
97:
98: count = this_cpu_read(*fbc->counters);
99: do {
100: if (unlikely(abs(count + amount) >= batch)) {
101: raw_spin_lock_irqsave(&fbc->lock, flags);
102: /*
103: * Note: by now we might have migrated to another CPU
104: * or the value might have changed.
105: */
106: count = __this_cpu_read(*fbc->counters);
107: fbc->count += count + amount;
108: __this_cpu_sub(*fbc->counters, count);
109: raw_spin_unlock_irqrestore(&fbc->lock, flags);
110: return;
111: }
112: } while (!this_cpu_try_cmpxchg(*fbc->counters, &count, count + amount));
113: }
SourceFile:lib/percpu_counter.c]
|
| 20/2 |
2026/04/05 16:35 |
llm |
expert |
0m
Model:
Tokens:
input: 15408
output: 43
thoughts: 1904
Thoughts: **Fprop Synchronization Analysis**
Okay, here's the breakdown of my thinking on this `flex_proportions` code and potential race conditions:
Initially, I see the core issue: We're dealing with an atomic counter (`percpu_counter`) update (`pl->events`) synchronized with a lock (`pl->lock`) on the writer side, and a lockless read of `pl->period` on the reader side. The critical question: if the reader sees a new `pl->period`, does it also *guaranteed* to see the updated `pl->events`?
The writer updates `pl->period` *after* a write to `pl->events` inside a locked critical section. The `raw_spin_unlock_irqrestore` provides a release barrier, meaning writes inside the critical section are visible to subsequent lock acquisitions. However, the reader *doesn't* acquire the lock. It does a simple comparison of `pl->period` against `period`.
My initial concern was reordering – could the reader see the new `pl->period` but not the "aged" `pl->events` counter update? This update, `percpu_counter_add_batch`, is essentially aging the counter. If the reader sees the *new* period, but doesn't "see" the updated events count, it would lead to a slightly incorrect proportion calculation.
I then considered the benignity of the error. This is writeback throttling, a heuristic. A *slight* error in proportions isn't catastrophic. It's a matter of performance tuning, not strict correctness.
Compiler optimizations could cause reloads, but the lock at line 118 and the check at line 120 protect against any multiple executions of the aging logic.
The "torn" access issue of reading a 4-byte `pl->period` came to mind, but on a modern architecture this is atomic. Even if it was torn, the lock guarantees consistency.
I then went through my mental checklist: is this like single-writer flag updates, or marked reloads? This seems more like a single writer flag update, where the check against the flag is a simple check.
The potential harm is delayed aging – the next event adds to the local and global counter, and the aging doesn't happen till the periodic check. The proportions would be less accurate but the reader *will* catch it on the next event.
Thinking about the code path, the reader returns if `pl->period == period`. I realized that this is actually *good*. If it thinks it's up to date, it returns. This means the lockless read is acting as a check. If it sees the new period, it *should* reflect the current state. The key issue is visibility and potential reordering of *writes* from the writer. The writer does, in sequence:
1. Age `pl->events`
2. Update `pl->period`
3. Unlock
The reader does:
1. Read `pl->period`
2. If the same, it returns.
3. Use `pl->events`
The problem is if the reader sees the updated `pl->period` at step 2, but the updated events are not yet visible to it.
On x86, loads are not reordered with other loads, and stores are not reordered with other stores. Since we have a writer with two stores (step 1 and 2), the stores should stay in the same order. Thus, if the reader sees the period, it should see events.
However, kernel code should be portable! On other architectures, this might be a problem. Even if this *is* a race, it only delays the aging for one event.
However, even if there *is* a problem, it's not a correctness bug; it's a slight performance degradation because the writeback throttling is slightly off.
Finally, I checked for other usages of `pl->period`. It seems to be only within the `fprop_reflect_period_percpu` function. Let's look at `fprop_new_period`. I will stop here, and look at the calls to that function!
|
| 21/2 |
2026/04/05 16:35 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_new_period]
Results: map[SourceCode: 56: /*
57: * Declare @periods new periods. It is upto the caller to make sure period
58: * transitions cannot happen in parallel.
59: *
60: * The function returns true if the proportions are still defined and false
61: * if aging zeroed out all events. This can be used to detect whether declaring
62: * further periods has any effect.
63: */
64: bool fprop_new_period(struct fprop_global *p, int periods)
65: {
66: s64 events = percpu_counter_sum(&p->events);
67: unsigned long flags;
68:
69: /*
70: * Don't do anything if there are no events.
71: */
72: if (events <= 1)
73: return false;
74: local_irq_save(flags);
75: write_seqcount_begin(&p->sequence);
76: if (periods < 64)
77: events -= events >> periods;
78: /* Use addition to avoid losing events happening between sum and set */
79: percpu_counter_add(&p->events, -events);
80: p->period += periods;
81: write_seqcount_end(&p->sequence);
82: local_irq_restore(flags);
83:
84: return true;
85: }
SourceFile:lib/flex_proportions.c]
|
| 22/2 |
2026/04/05 16:35 |
llm |
expert |
0m
Model:
Tokens:
input: 18673
output: 43
thoughts: 2847
Thoughts: **Analysis of Lockless Period Read in `fprop_reflect_period_percpu`**
Okay, here's my breakdown of this KCSAN report. The crux of the matter is the lockless read of `pl->period` in `fprop_reflect_period_percpu`. The `period` of both `fprop_global` and the per-CPU local `fprop_local_percpu` are being updated, and the race highlighted involves the reader potentially seeing a stale `pl->period`. This is a classic "double-checked locking" pattern, but without the appropriate memory barriers.
Initially, I'm thinking, "Is this actually *harmful*?". KCSAN flags this because, logically, it *requires* `READ_ONCE`, `WRITE_ONCE`, or similar annotations. The compiler *could* potentially misbehave, such as splitting the 32-bit write or hoisting the read, even though in the case of x86, it's unlikely to split the write since it's atomic.
The report mentions a single-bit change (0x2b to 0x2c), which makes me think there's little real harm, as on x86, it's atomic. However, if the reader sees a torn value, it takes the lock. Inside the lock, another read occurs, which is safe. My concern immediately turned to the possibility of a torn value *equal* to the `period` the reader is comparing against.
I went digging, and my thought process led me to examine all the variables and how they were being updated. I found that `p->period` is also updated locklessly, adding a race on this variable as well, although KCSAN did not highlight it. This variable gets written by `fprop_new_period` and read locklessly in `fprop_reflect_period_percpu`. This leads me to believe that the potential issues are compounded, but the key is how this plays out in the wider system.
The check `if (pl->period == period)` is the key here. The reader *does* have a fast path. If it sees the new value, it simply returns. If it sees the old value, it takes the lock. Inside the lock, we have another read and check `if (pl->period >= period)`. This second check mitigates some concerns, as it protects against double-aging.
I thought about compiler reordering. Is the compiler free to optimize away the lockless read by, say, reordering the read, or by hoisting it out of the loop in `fprop_fraction_percpu`? The code isn't a loop within the flagged function, but is called from `fprop_fraction_percpu`, which is a loop. This would be a problem if it wasn't for the `read_seqcount_begin/retry` loop. The sequence count prevents this sort of compiler optimization from being harmful.
I considered potential corruption scenarios, and I had to consider adjacent unsynchronized operations. The update to `pl->events` and `pl->period` together could present problems if the reader sees an out-of-date value of `pl->events` or `pl->period` (at least in my mind). I'm confident, however, that the updates to `pl->events` and the aging calculation are done under a lock. This means there's a benign data race.
I still don't see a clear path to crash or corruption. The `percpu_counter_add_batch` operations on `pl->events` are atomic, so at worst, there might be a small delay, but nothing serious. It looks like the double-checked locking, even without the explicit memory barriers, combined with the percpu counters, prevents inconsistencies. KCSAN is flagging the potential for a problem, as it should, but the risk appears to be low.
|
| 23/2 |
2026/04/05 16:35 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:percpu_counter_set]
Results: map[SourceCode: 60: void percpu_counter_set(struct percpu_counter *fbc, s64 amount)
61: {
62: int cpu;
63: unsigned long flags;
64:
65: raw_spin_lock_irqsave(&fbc->lock, flags);
66: for_each_possible_cpu(cpu) {
67: s32 *pcount = per_cpu_ptr(fbc->counters, cpu);
68: *pcount = 0;
69: }
70: fbc->count = amount;
71: raw_spin_unlock_irqrestore(&fbc->lock, flags);
72: }
SourceFile:lib/percpu_counter.c]
|
| 24/2 |
2026/04/05 16:35 |
llm |
expert |
0m
Model:
Tokens:
input: 22614
output: 46
thoughts: 2032
Thoughts: Okay, let me think this through.
**Analysis of a Potential Race Condition in Writeback Proportion Aging**
Right, so we're looking at a potential race between `percpu_counter_set` and `percpu_counter_add_batch`. `percpu_counter_set` takes a lock, zeroes out per-cpu counters, and is called when aging proportions, while `percpu_counter_add_batch` can take the lock in a slow path, or use a compare-and-swap in a fast path to increment a counter. The scenario is that `percpu_counter_set` could be zeroing a counter at the same time `percpu_counter_add_batch` is trying to increment it. If the `add` happens before the `set`, the value is lost. If after, it's preserved. However, `percpu_counter_set` is only called when aging the proportions, which are so old, that losing a single event wouldn't be a big deal.
Now, the code has a writer and a reader. The writer holds a lock, modifies `pl->period`, and calls `percpu_counter_set`, and the reader checks `pl->period` before updating. If the reader sees the new `pl->period`, it calls `percpu_counter_add_batch`. Crucially, the writer's `set` happens *before* the period update, so the reader's `add` *must* happen after the `set` if the reader sees the new period, and therefore no events should be lost.
Ah, but this depends on memory ordering. On x86, it's fine, stores are not reordered. On other architectures, we need to consider barriers. But the writer's lock release acts as a release barrier, and the `percpu_counter_set` uses `fbc->lock`. The write to `pl->period` is after the `fbc->lock` release, so the previous write should be visible. However, the reader doesn't use an acquire barrier, which is a potential issue.
So the reader *could* see the updated `pl->period` *before* seeing the zeroing of the counter, because the load could be reordered, but the writer is a single thread. It will execute things in order, and there is a subsequent lock operation for a memory barrier.
Even if there is a reordering here, it's a *heuristic* mechanism. It's not a crash, nor is it data corruption. I am leaning towards **BENIGN**, but it probably warrants the use of `READ_ONCE` or `WRITE_ONCE` to avoid any compiler optimizations that may alter the read or write sequence.
Wait, the "HARMFUL" criteria...if the race requires annotations for correctness, it's HARMFUL. And compiler behavior breaks it? Can the compiler break this particular algorithm? No, because it is not going to re-load `pl->period`, and the write is 32-bit. Fused accesses are unlikely here, and torn accesses are unlikely with a 32 bit word.
Okay, so I will stick with **BENIGN**. The Linux kernel is full of such races on statistics or heuristics. `flex_proportions` is definitely a heuristic.
Let's quickly check other uses of `fprop_reflect_period_percpu`. It's used in `fprop_fraction_percpu` and `__fprop_add_percpu` which are all about writeback proportions. I should also check what `pl->period` is *actually* used for. It is only used to determine if the counter is aged. If wrong, the counter ages incorrectly, but only affects how many events have happened in a period, which is only used in a proportion. This proportion is used to set the dirty limit for the BDI. If slightly off, it would only have sub-optimal writeback. No crash.
The stack traces confirm it's for page writeback statistics. The writer is calculating writeback thresholds. So it's all writeback heuristics.
Wait, I should check the "value changed" output in the report. This is a typical counter pattern.
I will conclude this as **BENIGN**. It's a race on a fast path, heuristic mechanism. The locking ensures that aging logic is synchronized. The lockless read *could* see a stale value which leads to taking the lock unnecessarily, but if it sees the new value, it is correct. The only risk is a torn read (unlikely), or compiler reordering (again, unlikely to cause harm).
But wait! Is `pl->period` *decreased* anywhere? No, it only *increases*. This makes the comparison at line 120 very robust.
And the check
|
| 25/2 |
2026/04/05 16:36 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:lib/flex_proportions.c IncludeLines:true Name:fprop_local_init_percpu]
Results: map[SourceCode: 92: int fprop_local_init_percpu(struct fprop_local_percpu *pl, gfp_t gfp)
93: {
94: int err;
95:
96: err = percpu_counter_init(&pl->events, 0, gfp);
97: if (err)
98: return err;
99: pl->period = 0;
100: raw_spin_lock_init(&pl->lock);
101: return 0;
102: }
SourceFile:lib/flex_proportions.c]
|
| 26/2 |
2026/04/05 16:36 |
llm |
expert |
0m
Model:
Tokens:
input: 25900
output: 23
thoughts: 188
Thoughts: Here's my summary, as I see it:
**Benign Concurrency Concern**
Okay, so the counter is initialized to zero, which makes sense. We're dealing with a fast-path check for a monotonically increasing value, which is pretty standard in these scenarios. The logic itself seems robust; it's designed to handle potentially stale or torn reads, and that double-check within the lock guarantees correctness. The worst-case scenario with a race condition is either an unnecessary lock acquisition (slightly less performant) or a small delay in the aging process. On x86, torn reads are very unlikely, and even if they happen on other architectures, it's not a critical error. **I'm going to classify this as BENIGN.**
But, hold on. Before I sign off on this, I need to be sure. Let's see if anyone else has reported anything similar. I'll search for references to `fprop_reflect_period_percpu` in conjunction with `KCSAN`. Unfortunately, I don't have my normal tooling available, but I can quickly scan the recent commit history in `lib/flex_proportions.c` just in case.
|
| 27/2 |
2026/04/05 16:36 |
tool |
grepper |
0m
Args: map[Expression:fprop_reflect_period_percpu]
Results: map[Output:lib/flex_proportions.c=104=void fprop_local_destroy_percpu(struct fprop_local_percpu *pl)
--
lib/flex_proportions.c-108-
lib/flex_proportions.c:109:static void fprop_reflect_period_percpu(struct fprop_global *p,
lib/flex_proportions.c-110- struct fprop_local_percpu *pl)
--
lib/flex_proportions.c=140=void __fprop_add_percpu(struct fprop_global *p, struct fprop_local_percpu *pl,
--
lib/flex_proportions.c-142-{
lib/flex_proportions.c:143: fprop_reflect_period_percpu(p, pl);
lib/flex_proportions.c-144- percpu_counter_add_batch(&pl->events, nr, PROP_BATCH);
--
lib/flex_proportions.c=148=void fprop_fraction_percpu(struct fprop_global *p,
--
lib/flex_proportions.c-156- seq = read_seqcount_begin(&p->sequence);
lib/flex_proportions.c:157: fprop_reflect_period_percpu(p, pl);
lib/flex_proportions.c-158- num = percpu_counter_read_positive(&pl->events);
]
|
| 28/2 |
2026/04/05 16:36 |
llm |
expert |
0m
Model:
Tokens:
input: 26732
output: 24
thoughts: 1264
Thoughts: **My Analysis of the Potential Race Condition**
Okay, so I'm looking at this code and initially, it seems like the race condition is minimal, primarily a fast-path optimization. The code hasn't changed substantially, and the race exists, but it's designed around a fast path.
First thing, I need to verify what `pl->period` is used for. Alright, it's exclusively used within `fprop_reflect_period_percpu`. So, I'm leaning towards this being **BENIGN**. The kernel's full of these "benign" races in fast-path checks. The double-check within the lock guarantees the rightness of the overall process. The worst that could happen because of this race is a slightly inefficient fast-path (unnecessary lock use) or a minor delay in aging.
Now, I'm checking if `pl->period` is ever updated with something *other* than `p->period`. No, looking at line 135: `pl->period = period;` where `period = p->period;` (line 112). Therefore, `pl->period` always catches up.
The race is likely between two threads calling `fprop_reflect_period_percpu` on the same `pl`. One thread is updating `pl->period` (the writer), while the other is checking it without a lock (the reader). If the reader sees the old value, it grabs the lock. If it sees the new value, it just returns. Both are valid.
The real risk is a torn value. But 32-bit aligned reads are atomic on x86, which is good. Even if it's torn, the double-check in the lock addresses that. If the torn value is equal to the `period` (unlikely), it returns. But that `period` is itself changing!
Let me run a few hypothetical scenarios. What if `period` is 44 and `pl->period` is 43? The writer is updating `pl->period` to 44. The reader reads `pl->period` and sees 44. It returns. Correct. What if the reader reads 43? Grabs the lock. Inside the lock, sees 44 (writer finished). Returns. Also correct. What if the reader reads a torn value? Grabs the lock. Inside the lock, sees 44. Returns. Correct.
So, the only way it's wrong is if the reader sees 44 when it should have seen 43, *but* 44 is the "right" future value anyway! The only problem is if the reader sees 44 but `pl->events` hasn't been aged yet. But the writer holds the `pl->lock` while updating both. The writer updates `pl->events` (lines 131/134) *then* `pl->period` (line 135). Because it's a single thread, these updates happen in that sequence. On x86, stores aren't reordered. Therefore, if the reader sees the new `pl->period`, it *must* also see the new `pl->events`. Other architectures might not have the same guarantee, but it's not fatal.
Wait, should I check if `pl->events` is updated with `WRITE_ONCE` or anything similar? No, `percpu_counter_add_batch` and `percpu_counter_set` use locks or atomic operations. So the updates to `pl->events` are well-defined and handled in a safe way.
So, in conclusion, this seems **BENIGN**.
Ok, let me zoom out one more time. Checking the fields in `fprop_local_percpu`: just `events`, `period`, and `lock`. `events` is a `percpu_counter`. So, the only race is on `pl->period`.
**Final Verdict: BENIGN.** The race exists on a fast-path check for a monotonically increasing counter. The double-check inside the lock ensures the correctness of the code. The worst consequence is suboptimal fast-path use or a small delay. It is structurally fine with stale or torn reads.
I do, however, think it's worth making some recommendations. I'd add `READ_ONCE` for the lockless read at line 116. `WRITE_ONCE` for the write at line 135. Or `data_race()` if we want to just tell the compiler to ignore it. `READ_ONCE` / `WRITE_ONCE` are better to avoid compiler optimization issues.
Wait, the report says "value changed: 0x0000002b -> 0x00
|
| 29/2 |
2026/04/05 16:36 |
tool |
codesearch-find-references |
0m
Args: map[Name:fprop_new_period]
Results: map[References:[map[ReferenceKind:calls ReferencingEntityKind:function ReferencingEntityName:writeout_period SourceFile:mm/page-writeback.c SourceLine:607]] TruncatedOutput:false]
|
| 30/2 |
2026/04/05 16:36 |
llm |
expert |
0m
Model:
Tokens:
input: 29114
output: 45
thoughts: 138
Thoughts: Okay, let me break this down.
**Diving into the Writeback Process**
Alright, so `writeout_period` is definitely being invoked by `wb_domain_writeout_add`. That's the starting point. But wait a minute... the stack trace of the *reader* has `wb_domain_writeout_add` in it! That's unexpected. Let me look closer. Ah, there it is: `wb_domain_writeout_add mm/page-writeback.c:554 [inline]` and then `__wb_writeout_add+0x75/0x1b0 mm/page-writeback.c:578`. So that explains it.
Now, hold on. I need to be sure. I seem to recall `wb_domain_writeout_add` calling `fprop_new_period`. I need to confirm that's correct, so I'll check directly in `mm/page-writeback.c`.
|
| 31/2 |
2026/04/05 16:36 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:mm/page-writeback.c IncludeLines:true Name:wb_domain_writeout_add]
Results: map[SourceCode: 550: static void wb_domain_writeout_add(struct wb_domain *dom,
551: struct fprop_local_percpu *completions,
552: unsigned int max_prop_frac, long nr)
553: {
554: __fprop_add_percpu_max(&dom->completions, completions,
555: max_prop_frac, nr);
556: /* First event after period switching was turned off? */
557: if (unlikely(!dom->period_time)) {
558: /*
559: * We can race with other wb_domain_writeout_add calls here but
560: * it does not cause any harm since the resulting time when
561: * timer will fire and what is in writeout_period_time will be
562: * roughly the same.
563: */
564: dom->period_time = wp_next_time(jiffies);
565: mod_timer(&dom->period_timer, dom->period_time);
566: }
567: }
SourceFile:mm/page-writeback.c]
|
| 32/2 |
2026/04/05 16:36 |
llm |
expert |
0m
Model:
Tokens:
input: 29854
output: 41
thoughts: 71
Thoughts: Here's my thought process, as it stands:
**Navigating the `fprop` Code Path**
Okay, so I'm tracing through this `fprop` code, and I've hit a specific call: `__fprop_add_percpu_max`. Right, and *that* calls `fprop_reflect_period_percpu`. I remember seeing that one somewhere...
Now, a bit of a wrinkle – I'm recalling that `fprop_new_period` is actually invoked from `writeout_period`. So that's the link. So it makes sense to pause here and dig into `writeout_period`. That's likely where the connection is. Let me check that.
|
| 33/2 |
2026/04/05 16:36 |
tool |
codesearch-definition-source |
0m
Args: map[ContextFile:mm/page-writeback.c IncludeLines:true Name:writeout_period]
Results: map[SourceCode: 597: /*
598: * On idle system, we can be called long after we scheduled because we use
599: * deferred timers so count with missed periods.
600: */
601: static void writeout_period(struct timer_list *t)
602: {
603: struct wb_domain *dom = timer_container_of(dom, t, period_timer);
604: int miss_periods = (jiffies - dom->period_time) /
605: VM_COMPLETIONS_PERIOD_LEN;
606:
607: if (fprop_new_period(&dom->completions, miss_periods + 1)) {
608: dom->period_time = wp_next_time(dom->period_time +
609: miss_periods * VM_COMPLETIONS_PERIOD_LEN);
610: mod_timer(&dom->period_timer, dom->period_time);
611: } else {
612: /*
613: * Aging has zeroed all fractions. Stop wasting CPU on period
614: * updates.
615: */
616: dom->period_time = 0;
617: }
618: }
SourceFile:mm/page-writeback.c]
|
| 34/2 |
2026/04/05 16:36 |
llm |
expert |
3m
Model:
Tokens:
input: 30500
output: 19
thoughts: 62915
|
| 35/2 |
2026/04/05 16:40 |
tool |
set-results |
0m
Args: map[Benign:true Confident:true]
Results: map[Benign:true Confident:true]
|
| 36/2 |
2026/04/05 16:40 |
llm |
expert |
0m
Model:
Tokens:
input: 93451
output: 645
thoughts: 205
Thoughts: **Benign Data Race Analysis: A Personal Assessment**
Okay, the deep dive is done, and I've come to a firm conclusion: this data race is actually benign. I spotted it in the `fprop_reflect_period_percpu` function, specifically with the `pl->period` field. Here's the rundown of my thought process:
One thread, our writer, updates `pl->period` while it's holding `pl->lock`. Textbook synchronization, nothing unusual there. But here's where it gets interesting. Another thread, the reader, is trying to be clever and optimize things with a fast path: it checks `pl->period` *without* grabbing the lock.
Now, if the reader encounters a stale value, no big deal. It's smart enough to fall back, take the lock, and re-check `pl->period` within a critical section. This double-check pattern is key; it guarantees correctness in the worst-case scenario. If the reader sees the updated value during the initial check, it can correctly return early, avoiding the lock entirely.
The reason I'm confident this is harmless lies in the inherent structure of the data and the operation. `pl->period` is monotonically increasing, so a stale read just means the reader is working with a slightly older value. Couple this with the double-check strategy and I see that this race is structurally tolerant to both stale and torn reads. The impact of the race is minimal: a marginally sub-optimal fast-path—taking the lock when it wasn't strictly necessary—or a slight delay in the heuristic-based proportions, which is hardly a stability or correctness issue.
So, in short, while it *is* a data race, it's not one I'm losing any sleep over. I've considered all the angles, and it's not going to bite us.
|
| 37/1 |
2026/04/05 16:40 |
action |
format-explanation |
0m
Results: map[Explanation:The KCSAN data race report identifies a race on the `pl->period` field within
the `fprop_reflect_period_percpu` function in `lib/flex_proportions.c`. This
function is part of the "flexible proportions" mechanism used by the Linux
kernel to track and age statistics for writeback throttling.
### Race Analysis
The race occurs between:
1. **Writer (Task 5216):** Updating `pl->period = period;` at line 135 while
holding the `pl->lock` spinlock.
2. **Reader (Task 6848):** Performing a lockless fast-path check `if
(pl->period == period)` at line 116.
This is a classic **double-checked locking** (or lockless fast-path) pattern.
The logic works as follows:
- The reader checks `pl->period` locklessly. If it matches the current global
period, the local statistics are already up to date, and the function returns
immediately.
- If the values do not match, the reader acquires `pl->lock` and performs a
second check: `if (pl->period >= period)`. This second check, performed under
the lock, ensures that only one thread performs the aging logic for a given
period transition.
### Severity: **BENIGN**
The race is considered **BENIGN** for the following reasons:
1. **Structural Tolerance:** The algorithm is designed to handle stale reads at
line 116. A stale read simply causes the thread to take the spinlock and
discover the update via the second check at line 120.
2. **Monotonicity:** The period counter is monotonically increasing. This
ensures that the checks `==` and `>=` are robust against typical race
conditions.
3. **Heuristic Nature:** The `flex_proportions` mechanism is used for writeback
heuristics (calculating dirty limits). Even if a race resulted in a slightly
delayed aging of a counter (e.g., if a reader saw a new period but not the aged
counter value due to memory reordering on non-x86 architectures), the impact
would be a negligible, transient inaccuracy in writeback throttling, not a
kernel crash or data corruption.
4. **Atomicity:** On the reported x86_64 architecture, 32-bit aligned accesses
to `unsigned int` are atomic, preventing torn reads. Even on architectures where
this might not be guaranteed, the double-check under the lock would resolve any
inconsistency.
### Recommended Annotations
While the race is benign, it is good practice to mark such intentional races to
satisfy static analysis tools and prevent aggressive compiler optimizations:
- Use `READ_ONCE(pl->period)` for the lockless read at line 116.
- Use `WRITE_ONCE(pl->period, period)` for the update at line 135.
- Alternatively, the lockless read can be wrapped in `data_race()`.]
|