block/blk.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
blk_time_get_ns() populates the per-plug cached timestamp and then
returns it by re-reading the field:
if (!plug->cur_ktime) {
plug->cur_ktime = ktime_get_ns();
current->flags |= PF_BLOCK_TS;
}
return plug->cur_ktime;
This is problematic when the compiler emits the final
"return plug->cur_ktime" as a reload from memory, after PF_BLOCK_TS has
already been set.
Since the cached timestamp is now invalidated from finish_task_switch()
(fad156c2af22 "block: invalidate cached plug timestamp after task
switch"), a task preempted between setting PF_BLOCK_TS and that reload
has plug->cur_ktime zeroed by blk_plug_invalidate_ts() when it is
scheduled back in. The reload then returns 0.
A 0 handed back here is stored as a start timestamp -- e.g.
blk_account_io_start() writes it to rq->start_time_ns -- and later
subtracted from "now". blk_account_io_done() then adds (now - 0), i.e.
roughly the system uptime, to the per-group nsecs[] counters. On an
otherwise idle, healthy device this appears as sudden ~uptime-sized jumps
in the diskstats time fields (write_ticks/discard_ticks/time_in_queue).
The solution is to be explicit in our reads and writes to this field
that is preemption volatile. We also add a barrier() to ensure that any
setting of PF_BLOCK_TS is ordered to happen after the cur_ktime update.
This issue was discovered using AI-assisted kprobes looking for paths
that were leaking zeroed timestamps in a live system, based on the
observation that we were sometimes seeing uptime-sized jumps in kernel
exported counters. This was flagged by NodeDiskIOSaturation
prometheus alerts that started firing on all hosts post 7.1.3 kernel
upgrade, due to node-exporter now exporting a nonsensical
node_disk_io_time_weighted_seconds_total.
Fixes: fad156c2af22 ("block: invalidate cached plug timestamp after task switch")
Cc: stable@vger.kernel.org
Signed-off-by: Mike Waychison <mike@waychison.com>
Assisted-by: Claude:claude-opus-4.8
---
block/blk.h | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..17e03656ba9a 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -689,6 +689,7 @@ static inline int req_ref_read(struct request *req)
static inline u64 blk_time_get_ns(void)
{
struct blk_plug *plug = current->plug;
+ u64 now;
if (!plug || !in_task())
return ktime_get_ns();
@@ -697,12 +698,18 @@ static inline u64 blk_time_get_ns(void)
* 0 could very well be a valid time, but rather than flag "this is
* a valid timestamp" separately, just accept that we'll do an extra
* ktime_get_ns() if we just happen to get 0 as the current time.
+ *
+ * cur_ktime can be zeroed by pre-emption the moment PF_BLOCK_TS is set.
*/
- if (!plug->cur_ktime) {
- plug->cur_ktime = ktime_get_ns();
+ now = READ_ONCE(plug->cur_ktime);
+ if (!now) {
+ now = ktime_get_ns();
+ WRITE_ONCE(plug->cur_ktime, now);
+ /* Ensure PF_BLOCK_TS is set after cur_ktime. */
+ barrier();
current->flags |= PF_BLOCK_TS;
}
- return plug->cur_ktime;
+ return now;
}
static inline ktime_t blk_time_get(void)
--
2.47.3
On Wed, 15 Jul 2026 15:29:50 -0400, Mike Waychison wrote:
> blk_time_get_ns() populates the per-plug cached timestamp and then
> returns it by re-reading the field:
>
> if (!plug->cur_ktime) {
> plug->cur_ktime = ktime_get_ns();
> current->flags |= PF_BLOCK_TS;
> }
> return plug->cur_ktime;
>
> [...]
Applied, thanks!
[1/1] block: fix race in blk_time_get_ns() returning 0
commit: 691b052139c94ee6640ac39e0b764dd3867897c0
Best regards,
--
Jens Axboe
© 2016 - 2026 Red Hat, Inc.