| Message ID | 20221023080431.30893-1-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/vmscan: reduce double-check if kswapd is not able to sleep | expand |
On Sun, Oct 23, 2022 at 08:04:31AM +0000, Wei Yang wrote: >In function kswapd_try_to_sleep, there are two phases for kswapd to >sleep: > > * premature sleep > * fully sleep > >For each phase we need to check whether kswapd is fine to sleep. > >While if it doesn't pass the check for first phase, it is not necessary >to do the check again. > >Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >CC: Mel Gorman <mgorman@techsingularity.net> >CC: Daero Lee <skseofh@gmail.com> > >--- >The original thread is >https://lkml.kernel.org/lkml/20220106094650.GX3366@techsingularity.net/T/, >but seems no further following up. > >So I pick it up. > >Mel, > >I just see your mail, sorry for the late reply :-( >--- > mm/vmscan.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > >diff --git a/mm/vmscan.c b/mm/vmscan.c >index 04d8b88e5216..5a50b5908c4c 100644 >--- a/mm/vmscan.c >+++ b/mm/vmscan.c >@@ -7179,7 +7179,8 @@ static enum zone_type kswapd_highest_zoneidx(pg_data_t *pgdat, > static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_order, > unsigned int highest_zoneidx) > { >- long remaining = 0; >+ long remaining; Hmm... I am afraid remaining should still be init to 0, otherwise count_vm_event() may record a wrong event.
Hi Wei,
url: https://github.com/intel-lab-lkp/linux/commits/Wei-Yang/mm-vmscan-reduce-double-check-if-kswapd-is-not-able-to-sleep/20221023-160625
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20221023080431.30893-1-richard.weiyang%40gmail.com
patch subject: [PATCH] mm/vmscan: reduce double-check if kswapd is not able to sleep
config: microblaze-randconfig-m041-20221023
compiler: microblaze-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
mm/vmscan.c:7261 kswapd_try_to_sleep() error: uninitialized symbol 'remaining'.
vim +/remaining +7261 mm/vmscan.c
38087d9b036098 Mel Gorman 2016-07-28 7179 static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_order,
97a225e69a1f88 Joonsoo Kim 2020-06-03 7180 unsigned int highest_zoneidx)
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7181 {
de349a300443b3 Wei Yang 2022-10-23 7182 long remaining;
de349a300443b3 Wei Yang 2022-10-23 7183 bool can_sleep;
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7184 DEFINE_WAIT(wait);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7185
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7186 if (freezing(current) || kthread_should_stop())
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7187 return;
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7188
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7189 prepare_to_wait(&pgdat->kswapd_wait, &wait, TASK_INTERRUPTIBLE);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7190
333b0a459c0e1b Shantanu Goel 2017-05-03 7191 /*
333b0a459c0e1b Shantanu Goel 2017-05-03 7192 * Try to sleep for a short interval. Note that kcompactd will only be
333b0a459c0e1b Shantanu Goel 2017-05-03 7193 * woken if it is possible to sleep for a short interval. This is
333b0a459c0e1b Shantanu Goel 2017-05-03 7194 * deliberate on the assumption that if reclaim cannot keep an
333b0a459c0e1b Shantanu Goel 2017-05-03 7195 * eligible zone balanced that it's also unlikely that compaction will
333b0a459c0e1b Shantanu Goel 2017-05-03 7196 * succeed.
333b0a459c0e1b Shantanu Goel 2017-05-03 7197 */
de349a300443b3 Wei Yang 2022-10-23 7198 can_sleep = prepare_kswapd_sleep(pgdat, reclaim_order, highest_zoneidx);
de349a300443b3 Wei Yang 2022-10-23 7199 if (can_sleep) {
fd901c95388b3b Vlastimil Babka 2016-04-28 7200 /*
fd901c95388b3b Vlastimil Babka 2016-04-28 7201 * Compaction records what page blocks it recently failed to
fd901c95388b3b Vlastimil Babka 2016-04-28 7202 * isolate pages from and skips them in the future scanning.
fd901c95388b3b Vlastimil Babka 2016-04-28 7203 * When kswapd is going to sleep, it is reasonable to assume
fd901c95388b3b Vlastimil Babka 2016-04-28 7204 * that pages and compaction may succeed so reset the cache.
fd901c95388b3b Vlastimil Babka 2016-04-28 7205 */
fd901c95388b3b Vlastimil Babka 2016-04-28 7206 reset_isolation_suitable(pgdat);
fd901c95388b3b Vlastimil Babka 2016-04-28 7207
fd901c95388b3b Vlastimil Babka 2016-04-28 7208 /*
fd901c95388b3b Vlastimil Babka 2016-04-28 7209 * We have freed the memory, now we should compact it to make
fd901c95388b3b Vlastimil Babka 2016-04-28 7210 * allocation of the requested order possible.
fd901c95388b3b Vlastimil Babka 2016-04-28 7211 */
97a225e69a1f88 Joonsoo Kim 2020-06-03 7212 wakeup_kcompactd(pgdat, alloc_order, highest_zoneidx);
fd901c95388b3b Vlastimil Babka 2016-04-28 7213
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7214 remaining = schedule_timeout(HZ/10);
"remaining" only set when can_sleep is true.
38087d9b036098 Mel Gorman 2016-07-28 7215
38087d9b036098 Mel Gorman 2016-07-28 7216 /*
97a225e69a1f88 Joonsoo Kim 2020-06-03 7217 * If woken prematurely then reset kswapd_highest_zoneidx and
38087d9b036098 Mel Gorman 2016-07-28 7218 * order. The values will either be from a wakeup request or
38087d9b036098 Mel Gorman 2016-07-28 7219 * the previous request that slept prematurely.
38087d9b036098 Mel Gorman 2016-07-28 7220 */
38087d9b036098 Mel Gorman 2016-07-28 7221 if (remaining) {
97a225e69a1f88 Joonsoo Kim 2020-06-03 7222 WRITE_ONCE(pgdat->kswapd_highest_zoneidx,
97a225e69a1f88 Joonsoo Kim 2020-06-03 7223 kswapd_highest_zoneidx(pgdat,
97a225e69a1f88 Joonsoo Kim 2020-06-03 7224 highest_zoneidx));
5644e1fbbfe15a Qian Cai 2020-04-01 7225
5644e1fbbfe15a Qian Cai 2020-04-01 7226 if (READ_ONCE(pgdat->kswapd_order) < reclaim_order)
5644e1fbbfe15a Qian Cai 2020-04-01 7227 WRITE_ONCE(pgdat->kswapd_order, reclaim_order);
de349a300443b3 Wei Yang 2022-10-23 7228 can_sleep = false;
de349a300443b3 Wei Yang 2022-10-23 7229 } else {
de349a300443b3 Wei Yang 2022-10-23 7230 can_sleep = prepare_kswapd_sleep(pgdat, reclaim_order,
de349a300443b3 Wei Yang 2022-10-23 7231 highest_zoneidx);
38087d9b036098 Mel Gorman 2016-07-28 7232 }
38087d9b036098 Mel Gorman 2016-07-28 7233
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7234 finish_wait(&pgdat->kswapd_wait, &wait);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7235 prepare_to_wait(&pgdat->kswapd_wait, &wait, TASK_INTERRUPTIBLE);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7236 }
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7237
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7238 /*
de349a300443b3 Wei Yang 2022-10-23 7239 * If kswapd is fine to sleep, restore vmstat thresholds and kswapd
de349a300443b3 Wei Yang 2022-10-23 7240 * goes to sleep.
de349a300443b3 Wei Yang 2022-10-23 7241 * If not, account whether the low or high watermark was hit quickly.
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7242 */
de349a300443b3 Wei Yang 2022-10-23 7243 if (can_sleep) {
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7244 trace_mm_vmscan_kswapd_sleep(pgdat->node_id);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7245
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7246 /*
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7247 * vmstat counters are not perfectly accurate and the estimated
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7248 * value for counters such as NR_FREE_PAGES can deviate from the
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7249 * true value by nr_online_cpus * threshold. To avoid the zone
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7250 * watermarks being breached while under pressure, we reduce the
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7251 * per-cpu vmstat threshold while kswapd is awake and restore
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7252 * them before going back to sleep.
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7253 */
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7254 set_pgdat_percpu_threshold(pgdat, calculate_normal_threshold);
1c7e7f6c0703d0 Aaditya Kumar 2012-07-17 7255
1c7e7f6c0703d0 Aaditya Kumar 2012-07-17 7256 if (!kthread_should_stop())
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7257 schedule();
1c7e7f6c0703d0 Aaditya Kumar 2012-07-17 7258
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7259 set_pgdat_percpu_threshold(pgdat, calculate_pressure_threshold);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7260 } else {
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 @7261 if (remaining)
Uninitialized here.
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7262 count_vm_event(KSWAPD_LOW_WMARK_HIT_QUICKLY);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7263 else
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7264 count_vm_event(KSWAPD_HIGH_WMARK_HIT_QUICKLY);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7265 }
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7266 finish_wait(&pgdat->kswapd_wait, &wait);
f0bc0a60b13f20 KOSAKI Motohiro 2011-01-13 7267 }
On Mon, Oct 24, 2022 at 10:04:31AM +0300, Dan Carpenter wrote: >Hi Wei, > >url: https://github.com/intel-lab-lkp/linux/commits/Wei-Yang/mm-vmscan-reduce-double-check-if-kswapd-is-not-able-to-sleep/20221023-160625 >base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything >patch link: https://lore.kernel.org/r/20221023080431.30893-1-richard.weiyang%40gmail.com >patch subject: [PATCH] mm/vmscan: reduce double-check if kswapd is not able to sleep >config: microblaze-randconfig-m041-20221023 >compiler: microblaze-linux-gcc (GCC) 12.1.0 > >If you fix the issue, kindly add following tag where applicable >| Reported-by: kernel test robot <lkp@intel.com> >| Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > >New smatch warnings: >mm/vmscan.c:7261 kswapd_try_to_sleep() error: uninitialized symbol 'remaining'. > Thanks, I noticed the issue :-) >vim +/remaining +7261 mm/vmscan.c >
diff --git a/mm/vmscan.c b/mm/vmscan.c index 04d8b88e5216..5a50b5908c4c 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -7179,7 +7179,8 @@ static enum zone_type kswapd_highest_zoneidx(pg_data_t *pgdat, static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_order, unsigned int highest_zoneidx) { - long remaining = 0; + long remaining; + bool can_sleep; DEFINE_WAIT(wait); if (freezing(current) || kthread_should_stop()) @@ -7194,7 +7195,8 @@ static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_o * eligible zone balanced that it's also unlikely that compaction will * succeed. */ - if (prepare_kswapd_sleep(pgdat, reclaim_order, highest_zoneidx)) { + can_sleep = prepare_kswapd_sleep(pgdat, reclaim_order, highest_zoneidx); + if (can_sleep) { /* * Compaction records what page blocks it recently failed to * isolate pages from and skips them in the future scanning. @@ -7223,6 +7225,10 @@ static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_o if (READ_ONCE(pgdat->kswapd_order) < reclaim_order) WRITE_ONCE(pgdat->kswapd_order, reclaim_order); + can_sleep = false; + } else { + can_sleep = prepare_kswapd_sleep(pgdat, reclaim_order, + highest_zoneidx); } finish_wait(&pgdat->kswapd_wait, &wait); @@ -7230,11 +7236,11 @@ static void kswapd_try_to_sleep(pg_data_t *pgdat, int alloc_order, int reclaim_o } /* - * After a short sleep, check if it was a premature sleep. If not, then - * go fully to sleep until explicitly woken up. + * If kswapd is fine to sleep, restore vmstat thresholds and kswapd + * goes to sleep. + * If not, account whether the low or high watermark was hit quickly. */ - if (!remaining && - prepare_kswapd_sleep(pgdat, reclaim_order, highest_zoneidx)) { + if (can_sleep) { trace_mm_vmscan_kswapd_sleep(pgdat->node_id); /*
In function kswapd_try_to_sleep, there are two phases for kswapd to sleep: * premature sleep * fully sleep For each phase we need to check whether kswapd is fine to sleep. While if it doesn't pass the check for first phase, it is not necessary to do the check again. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> CC: Mel Gorman <mgorman@techsingularity.net> CC: Daero Lee <skseofh@gmail.com> --- The original thread is https://lkml.kernel.org/lkml/20220106094650.GX3366@techsingularity.net/T/, but seems no further following up. So I pick it up. Mel, I just see your mail, sorry for the late reply :-( --- mm/vmscan.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)