Re: Re: Re: "mm: move pcp and lru-pcp draining into single wq" broke resume from s2ram

[Tetsuo Handa]

> On Wed 19-04-17 14:41:30, Tetsuo Handa wrote:
> [...]
> > Somebody is waiting forever with cpu_hotplug.lock held?
> 
> Why would that matter for drain_all_pages? It doesn't use
> get_online_cpus since a459eeb7b852 ("mm, page_alloc: do not depend on
> cpu hotplug locks inside the allocator") while ce612879ddc7 ("mm: move
> pcp and lru-pcp draining into single wq") was merged later.
> 

Looking at ce612879ddc7 ("mm: move pcp and lru-pcp draining into single wq"),
we merged "lru-add-drain" (!WQ_FREEZABLE && WQ_MEM_RECLAIM) workqueue and
"vmstat" (WQ_FREEZABLE && WQ_MEM_RECLAIM) workqueue into
"mm_percpu_wq" (WQ_FREEZABLE && WQ_MEM_RECLAIM) workqueue.

-   lru_add_drain_wq = alloc_workqueue("lru-add-drain", WQ_MEM_RECLAIM, 0);
-   vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
+   mm_percpu_wq = alloc_workqueue("mm_percpu_wq",
+  WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);

That means "lru-add-drain" became freezable, doesn't it? And this problem
occurs around resume operation where all freezable threads are frozen?
Then, lru_add_drain_per_cpu() cannot be performed due to mm_percpu_wq frozen?

Re: Re: "mm: move pcp and lru-pcp draining into single wq" broke resume from s2ram

[Tetsuo Handa]

Geert Uytterhoeven wrote:
> 8 locks held by s2ram/1899:
>  #0:  (sb_writers#7){.+.+.+}, at: [] vfs_write+0xa8/0x15c
>  #1:  (>mutex){+.+.+.}, at: [] 
> kernfs_fop_write+0xf0/0x194
>  #2:  (s_active#48){.+.+.+}, at: [] 
> kernfs_fop_write+0xf8/0x194
>  #3:  (pm_mutex){+.+.+.}, at: [] pm_suspend+0x16c/0xabc
>  #4:  (>mutex){..}, at: [] device_resume+0x58/0x190
>  #5:  (cma_mutex){+.+...}, at: [] cma_alloc+0x150/0x374
>  #6:  (lock){+.+...}, at: [] lru_add_drain_all+0x4c/0x1b4
>  #7:  (cpu_hotplug.dep_map){++}, at: [] 
> get_online_cpus+0x3c/0x9c

I think this situation suggests that

int pm_suspend(suspend_state_t state) {
  error = enter_state(state) {
if (!mutex_trylock(_mutex)) /* #3 */
  return -EBUSY;
error = suspend_devices_and_enter(state) {
  error = suspend_enter(state, ) {
enable_nonboot_cpus() {
  cpu_maps_update_begin() {
mutex_lock(_add_remove_lock);
  }
  pr_info("Enabling non-boot CPUs ...\n");
  for_each_cpu(cpu, frozen_cpus) {
error = _cpu_up(cpu, 1, CPUHP_ONLINE) {
  cpu_hotplug_begin() {
mutex_lock(_hotplug.lock);
  }
  
  cpu_hotplug_done() {
mutex_unlock(_hotplug.lock);
  }
}
if (!error) {
  pr_info("CPU%d is up\n", cpu);
  continue;
}
  }
  cpu_maps_update_done() {
 mutex_unlock(_add_remove_lock);
  }
}
  }
  dpm_resume_end(PMSG_RESUME) {
dpm_resume(state) {
  mutex_lock(_list_mtx);
  while (!list_empty(_suspended_list)) {
mutex_unlock(_list_mtx);
error = device_resume(dev, state, false) {
  dpm_wait_for_superior(dev, async);
  dpm_watchdog_set(, dev);
  device_lock(dev) {
mutex_lock(>mutex); /* #4 */
  }
  error = dpm_run_callback(callback, dev, state, info) {
cma_alloc() {
  mutex_lock(_mutex); /* #5 */
  alloc_contig_range() {
lru_add_drain_all() {
  mutex_lock(); /* #6 */
  get_online_cpus() {
mutex_lock(_hotplug.lock); /* #7 hang? */
mutex_unlock(_hotplug.lock);
  }
  put_online_cpus();
  mutex_unlock(); /* #6 */
}
  }
  mutex_unlock(_mutex); /* #5 */
}
  }
  device_unlock(dev) {
mutex_unlock(>mutex); /* #4 */
  }
}
mutex_lock(_list_mtx);
  }
  mutex_unlock(_list_mtx);
}
dpm_complete(state) {
  mutex_lock(_list_mtx);
  while (!list_empty(_prepared_list)) {
mutex_unlock(_list_mtx);
device_complete(dev, state) {
}
mutex_lock(_list_mtx);
  }
  mutex_unlock(_list_mtx);
}
  }
}
mutex_unlock(_mutex); /* #3 */
  }
}

Somebody is waiting forever with cpu_hotplug.lock held?
I think that full dmesg with SysRq-t output is appreciated.