Commit 20792ebf authored by Jan Kara's avatar Jan Kara Committed by Linus Torvalds

writeback: use READ_ONCE for unlocked reads of writeback stats

We do some unlocked reads of writeback statistics like
avg_write_bandwidth, dirty_ratelimit, or bw_time_stamp.  Generally we are
fine with getting somewhat out-of-date values but actually getting
different values in various parts of the functions because the compiler
decided to reload value from original memory location could confuse
calculations.  Use READ_ONCE for these unlocked accesses and WRITE_ONCE
for the updates to be on the safe side.

Link: https://lkml.kernel.org/r/20210713104716.22868-5-jack@suse.czSigned-off-by: default avatarJan Kara <jack@suse.cz>
Cc: Michael Stapelberg <stapelberg+linux@google.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 42dd235c
...@@ -183,7 +183,7 @@ static struct fprop_local_percpu *wb_memcg_completions(struct bdi_writeback *wb) ...@@ -183,7 +183,7 @@ static struct fprop_local_percpu *wb_memcg_completions(struct bdi_writeback *wb)
static void wb_min_max_ratio(struct bdi_writeback *wb, static void wb_min_max_ratio(struct bdi_writeback *wb,
unsigned long *minp, unsigned long *maxp) unsigned long *minp, unsigned long *maxp)
{ {
unsigned long this_bw = wb->avg_write_bandwidth; unsigned long this_bw = READ_ONCE(wb->avg_write_bandwidth);
unsigned long tot_bw = atomic_long_read(&wb->bdi->tot_write_bandwidth); unsigned long tot_bw = atomic_long_read(&wb->bdi->tot_write_bandwidth);
unsigned long long min = wb->bdi->min_ratio; unsigned long long min = wb->bdi->min_ratio;
unsigned long long max = wb->bdi->max_ratio; unsigned long long max = wb->bdi->max_ratio;
...@@ -892,7 +892,7 @@ static long long pos_ratio_polynom(unsigned long setpoint, ...@@ -892,7 +892,7 @@ static long long pos_ratio_polynom(unsigned long setpoint,
static void wb_position_ratio(struct dirty_throttle_control *dtc) static void wb_position_ratio(struct dirty_throttle_control *dtc)
{ {
struct bdi_writeback *wb = dtc->wb; struct bdi_writeback *wb = dtc->wb;
unsigned long write_bw = wb->avg_write_bandwidth; unsigned long write_bw = READ_ONCE(wb->avg_write_bandwidth);
unsigned long freerun = dirty_freerun_ceiling(dtc->thresh, dtc->bg_thresh); unsigned long freerun = dirty_freerun_ceiling(dtc->thresh, dtc->bg_thresh);
unsigned long limit = hard_dirty_limit(dtc_dom(dtc), dtc->thresh); unsigned long limit = hard_dirty_limit(dtc_dom(dtc), dtc->thresh);
unsigned long wb_thresh = dtc->wb_thresh; unsigned long wb_thresh = dtc->wb_thresh;
...@@ -1115,7 +1115,7 @@ static void wb_update_write_bandwidth(struct bdi_writeback *wb, ...@@ -1115,7 +1115,7 @@ static void wb_update_write_bandwidth(struct bdi_writeback *wb,
&wb->bdi->tot_write_bandwidth) <= 0); &wb->bdi->tot_write_bandwidth) <= 0);
} }
wb->write_bandwidth = bw; wb->write_bandwidth = bw;
wb->avg_write_bandwidth = avg; WRITE_ONCE(wb->avg_write_bandwidth, avg);
} }
static void update_dirty_limit(struct dirty_throttle_control *dtc) static void update_dirty_limit(struct dirty_throttle_control *dtc)
...@@ -1324,7 +1324,7 @@ static void wb_update_dirty_ratelimit(struct dirty_throttle_control *dtc, ...@@ -1324,7 +1324,7 @@ static void wb_update_dirty_ratelimit(struct dirty_throttle_control *dtc,
else else
dirty_ratelimit -= step; dirty_ratelimit -= step;
wb->dirty_ratelimit = max(dirty_ratelimit, 1UL); WRITE_ONCE(wb->dirty_ratelimit, max(dirty_ratelimit, 1UL));
wb->balanced_dirty_ratelimit = balanced_dirty_ratelimit; wb->balanced_dirty_ratelimit = balanced_dirty_ratelimit;
trace_bdi_dirty_ratelimit(wb, dirty_rate, task_ratelimit); trace_bdi_dirty_ratelimit(wb, dirty_rate, task_ratelimit);
...@@ -1369,7 +1369,7 @@ static void __wb_update_bandwidth(struct dirty_throttle_control *gdtc, ...@@ -1369,7 +1369,7 @@ static void __wb_update_bandwidth(struct dirty_throttle_control *gdtc,
wb->dirtied_stamp = dirtied; wb->dirtied_stamp = dirtied;
wb->written_stamp = written; wb->written_stamp = written;
wb->bw_time_stamp = now; WRITE_ONCE(wb->bw_time_stamp, now);
spin_unlock(&wb->list_lock); spin_unlock(&wb->list_lock);
} }
...@@ -1393,7 +1393,7 @@ static void wb_bandwidth_estimate_start(struct bdi_writeback *wb) ...@@ -1393,7 +1393,7 @@ static void wb_bandwidth_estimate_start(struct bdi_writeback *wb)
spin_lock(&wb->list_lock); spin_lock(&wb->list_lock);
wb->dirtied_stamp = wb_stat(wb, WB_DIRTIED); wb->dirtied_stamp = wb_stat(wb, WB_DIRTIED);
wb->written_stamp = wb_stat(wb, WB_WRITTEN); wb->written_stamp = wb_stat(wb, WB_WRITTEN);
wb->bw_time_stamp = now; WRITE_ONCE(wb->bw_time_stamp, now);
spin_unlock(&wb->list_lock); spin_unlock(&wb->list_lock);
} }
} }
...@@ -1418,7 +1418,7 @@ static unsigned long dirty_poll_interval(unsigned long dirty, ...@@ -1418,7 +1418,7 @@ static unsigned long dirty_poll_interval(unsigned long dirty,
static unsigned long wb_max_pause(struct bdi_writeback *wb, static unsigned long wb_max_pause(struct bdi_writeback *wb,
unsigned long wb_dirty) unsigned long wb_dirty)
{ {
unsigned long bw = wb->avg_write_bandwidth; unsigned long bw = READ_ONCE(wb->avg_write_bandwidth);
unsigned long t; unsigned long t;
/* /*
...@@ -1440,8 +1440,8 @@ static long wb_min_pause(struct bdi_writeback *wb, ...@@ -1440,8 +1440,8 @@ static long wb_min_pause(struct bdi_writeback *wb,
unsigned long dirty_ratelimit, unsigned long dirty_ratelimit,
int *nr_dirtied_pause) int *nr_dirtied_pause)
{ {
long hi = ilog2(wb->avg_write_bandwidth); long hi = ilog2(READ_ONCE(wb->avg_write_bandwidth));
long lo = ilog2(wb->dirty_ratelimit); long lo = ilog2(READ_ONCE(wb->dirty_ratelimit));
long t; /* target pause */ long t; /* target pause */
long pause; /* estimated next pause */ long pause; /* estimated next pause */
int pages; /* target nr_dirtied_pause */ int pages; /* target nr_dirtied_pause */
...@@ -1721,12 +1721,12 @@ static void balance_dirty_pages(struct bdi_writeback *wb, ...@@ -1721,12 +1721,12 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
if (dirty_exceeded && !wb->dirty_exceeded) if (dirty_exceeded && !wb->dirty_exceeded)
wb->dirty_exceeded = 1; wb->dirty_exceeded = 1;
if (time_is_before_jiffies(wb->bw_time_stamp + if (time_is_before_jiffies(READ_ONCE(wb->bw_time_stamp) +
BANDWIDTH_INTERVAL)) BANDWIDTH_INTERVAL))
__wb_update_bandwidth(gdtc, mdtc, true); __wb_update_bandwidth(gdtc, mdtc, true);
/* throttle according to the chosen dtc */ /* throttle according to the chosen dtc */
dirty_ratelimit = wb->dirty_ratelimit; dirty_ratelimit = READ_ONCE(wb->dirty_ratelimit);
task_ratelimit = ((u64)dirty_ratelimit * sdtc->pos_ratio) >> task_ratelimit = ((u64)dirty_ratelimit * sdtc->pos_ratio) >>
RATELIMIT_CALC_SHIFT; RATELIMIT_CALC_SHIFT;
max_pause = wb_max_pause(wb, sdtc->wb_dirty); max_pause = wb_max_pause(wb, sdtc->wb_dirty);
...@@ -2376,7 +2376,8 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc) ...@@ -2376,7 +2376,8 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
* but if there's constant writeback being submitted, this makes sure * but if there's constant writeback being submitted, this makes sure
* writeback bandwidth is updated once in a while. * writeback bandwidth is updated once in a while.
*/ */
if (time_is_before_jiffies(wb->bw_time_stamp + BANDWIDTH_INTERVAL)) if (time_is_before_jiffies(READ_ONCE(wb->bw_time_stamp) +
BANDWIDTH_INTERVAL))
wb_update_bandwidth(wb); wb_update_bandwidth(wb);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment