From: shechenglong shechenglong@xfusion.com
[ Upstream commit 04bdb1a04d8a2a89df504c1e34250cd3c6e31a1c ]
Route bfqg_stats_add_aux() time accumulation into the destination stats object instead of the source, aligning with other stat fields.
Reviewed-by: Yu Kuai yukuai@fnnas.com Signed-off-by: shechenglong shechenglong@xfusion.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis of Commit: block,bfq: fix aux stat accumulation destination
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly indicates this is a bug fix: "fix aux stat accumulation destination". It describes that `bfqg_stats_add_aux()` was routing time accumulation into the wrong destination (source instead of destination). The fix aligns this field with all other stat fields in the same function.
- Has `Reviewed-by: Yu Kuai` - BFQ subsystem expertise - Signed off by Jens Axboe (block subsystem maintainer)
### 2. CODE CHANGE ANALYSIS
The change is a single-line fix in `block/bfq-cgroup.c`:
```c - bfq_stat_add_aux(&from->time, &from->time); + bfq_stat_add_aux(&to->time, &from->time); ```
**The Bug:** The function `bfqg_stats_add_aux()` is documented with `/* @to += @from */` - it should add stats FROM source TO destination. The buggy line was adding `from->time` to itself (`&from->time, &from->time`), which is clearly wrong.
**Root Cause:** A simple typo - `from` was used instead of `to` for the first argument.
**Pattern Evidence:** Looking at surrounding code, every other line follows the correct pattern: - `blkg_rwstat_add_aux(&to->merged, &from->merged)` - `blkg_rwstat_add_aux(&to->service_time, &from->service_time)` - `bfq_stat_add_aux(&to->avg_queue_size_sum, &from->avg_queue_size_sum)` - etc.
Only `time` had the incorrect `&from` as the first argument.
### 3. CLASSIFICATION
- **Bug type:** Logic error (typo) causing incorrect stat accumulation - **Category:** Clear bug fix - not a feature, not a cleanup - **Subsystem:** BFQ I/O scheduler cgroup support (debug statistics)
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed:** 1 line, essentially a single character change (`from` → `to`) - **Files touched:** 1 file - **Risk level:** Extremely LOW - **Code location:** Under `CONFIG_BFQ_CGROUP_DEBUG` - debug statistics only
This is about as surgical as a fix can get. Even if the fix were somehow wrong (which it clearly isn't), it only affects debug statistics output, not actual I/O scheduling behavior.
### 5. USER IMPACT
- **Affected users:** Those using BFQ I/O scheduler with cgroup debugging enabled - **Symptom:** The `time` statistic would be lost (not transferred to parent) when a BFQ cgroup is destroyed, causing inaccurate cumulative statistics - **Severity:** Low to moderate - affects accuracy of debug/monitoring data, not data integrity or system stability
### 6. STABILITY INDICATORS
- Reviewed by Yu Kuai (BFQ expert) - Accepted by Jens Axboe (block maintainer) - Trivially correct fix - the pattern is obvious from surrounding code
### 7. DEPENDENCY CHECK
- **Dependencies:** None - this is a self-contained fix - **Affected code existence:** BFQ cgroup code has existed in stable trees for years - **Clean backport:** Should apply cleanly to any kernel with BFQ cgroup support
### VERDICT
**Pros:** - Obviously correct fix (typo/copy-paste error) - Minimal change (1 line) - Zero regression risk - Fixes incorrect behavior in statistics accumulation - Well-reviewed and accepted by maintainers - No dependencies on other commits
**Cons:** - Only affects debug code (`CONFIG_BFQ_CGROUP_DEBUG`) - Low-impact bug (statistics accuracy, not data/system integrity)
This commit meets all stable kernel criteria: 1. ✅ Obviously correct - trivially evident typo fix 2. ✅ Fixes a real bug - stats were accumulated to wrong destination 3. ✅ Small and contained - single line change 4. ✅ No new features - pure bug fix 5. ✅ Should apply cleanly to stable
While the impact is limited to debug statistics users, the fix is so trivially correct and low-risk that there's no reason not to backport it. Users relying on BFQ cgroup statistics for monitoring or debugging would benefit from accurate data.
**YES**
block/bfq-cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 9fb9f3533150..6a75fe1c7a5c 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -380,7 +380,7 @@ static void bfqg_stats_add_aux(struct bfqg_stats *to, struct bfqg_stats *from) blkg_rwstat_add_aux(&to->merged, &from->merged); blkg_rwstat_add_aux(&to->service_time, &from->service_time); blkg_rwstat_add_aux(&to->wait_time, &from->wait_time); - bfq_stat_add_aux(&from->time, &from->time); + bfq_stat_add_aux(&to->time, &from->time); bfq_stat_add_aux(&to->avg_queue_size_sum, &from->avg_queue_size_sum); bfq_stat_add_aux(&to->avg_queue_size_samples, &from->avg_queue_size_samples);