[rh7,1/2] mm/memcg: Don't enable interrupts too soon.

Submitted by Andrey Ryabinin on Oct. 30, 2017, 1:51 p.m.

Details

Message ID 20171030135124.19189-1-aryabinin@virtuozzo.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Andrey Ryabinin Oct. 30, 2017, 1:51 p.m.
When mem_cgroup_move_parent() moves huge page it disables interrupt:

	if (nr_pages > 1) {
		VM_BUG_ON_PAGE(!PageTransHuge(page), page);
		flags = compound_lock_irqsave(page);
	}

and calls:
	ret = mem_cgroup_move_account(page, nr_pages, ...

which does the following:

	local_irq_disable();
	mem_cgroup_charge_statistics(to, page, nr_pages);
	...
	local_irq_enable();

So the last local_irq_enable() enables irq too early, which may lead
to the deadlock. mem_cgroup_move_account() should use local_irq_save()/
local_irq_restore() primitives instead.

Found while investigating https://jira.sw.ru/browse/PSBM-76011
but unrelated.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 mm/memcontrol.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 239fbca70b59..efc455d8ca81 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3597,15 +3597,14 @@  static int mem_cgroup_move_account(struct page *page,
 
 	/* caller should have done css_get */
 	pc->mem_cgroup = to;
-	move_unlock_mem_cgroup(from, &flags);
+	spin_unlock(&from->move_lock);
 	ret = 0;
 
-	local_irq_disable();
 	mem_cgroup_charge_statistics(to, page, nr_pages);
 	memcg_check_events(to, page);
 	mem_cgroup_charge_statistics(from, page, -nr_pages);
 	memcg_check_events(from, page);
-	local_irq_enable();
+	local_irq_restore(flags);
 out_unlock:
 	unlock_page(page);
 out:

Comments

Konstantin Khorenko Oct. 30, 2017, 2:50 p.m.
Please consider to apply these patches a as ReadyKernel patch after they pass QA in the scope of vz7 update 6.

https://readykernel.com/

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 10/30/2017 04:51 PM, Andrey Ryabinin wrote:
> When mem_cgroup_move_parent() moves huge page it disables interrupt:
>
> 	if (nr_pages > 1) {
> 		VM_BUG_ON_PAGE(!PageTransHuge(page), page);
> 		flags = compound_lock_irqsave(page);
> 	}
>
> and calls:
> 	ret = mem_cgroup_move_account(page, nr_pages, ...
>
> which does the following:
>
> 	local_irq_disable();
> 	mem_cgroup_charge_statistics(to, page, nr_pages);
> 	...
> 	local_irq_enable();
>
> So the last local_irq_enable() enables irq too early, which may lead
> to the deadlock. mem_cgroup_move_account() should use local_irq_save()/
> local_irq_restore() primitives instead.
>
> Found while investigating https://jira.sw.ru/browse/PSBM-76011
> but unrelated.
>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  mm/memcontrol.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 239fbca70b59..efc455d8ca81 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3597,15 +3597,14 @@ static int mem_cgroup_move_account(struct page *page,
>
>  	/* caller should have done css_get */
>  	pc->mem_cgroup = to;
> -	move_unlock_mem_cgroup(from, &flags);
> +	spin_unlock(&from->move_lock);
>  	ret = 0;
>
> -	local_irq_disable();
>  	mem_cgroup_charge_statistics(to, page, nr_pages);
>  	memcg_check_events(to, page);
>  	mem_cgroup_charge_statistics(from, page, -nr_pages);
>  	memcg_check_events(from, page);
> -	local_irq_enable();
> +	local_irq_restore(flags);
>  out_unlock:
>  	unlock_page(page);
>  out:
>