Re: mm: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected in split_huge_page_to_list

From: Aneesh Kumar K.V
Date: Wed Jan 20 2016 - 00:46:51 EST


"Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> writes:

......

>
> I think this should fix the issue:
>
> From 10859758dadfa249616870f63c1636ec9857c501 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> Date: Mon, 18 Jan 2016 16:28:12 +0300
> Subject: [PATCH] thp: fix interrupt unsafe locking in split_huge_page()
>
> split_queue_lock can be taken from interrupt context in some cases, but
> I forgot to convert locking in split_huge_page() to interrupt-safe
> primitives.
>
> Let's fix this.

Can you add the stack trace from the problem reported to the commit
message ?. That will help in identifying the interrupt context call path
easily.

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> ---
> mm/huge_memory.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 50342eff7960..21fda6a10e89 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3357,6 +3357,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> struct anon_vma *anon_vma;
> int count, mapcount, ret;
> bool mlocked;
> + unsigned long flags;
>
> VM_BUG_ON_PAGE(is_huge_zero_page(page), page);
> VM_BUG_ON_PAGE(!PageAnon(page), page);
> @@ -3396,7 +3397,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> lru_add_drain();
>
> /* Prevent deferred_split_scan() touching ->_count */
> - spin_lock(&split_queue_lock);
> + spin_lock_irqsave(&split_queue_lock, flags);
> count = page_count(head);
> mapcount = total_mapcount(head);
> if (!mapcount && count == 1) {
> @@ -3404,11 +3405,11 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> split_queue_len--;
> list_del(page_deferred_list(head));
> }
> - spin_unlock(&split_queue_lock);
> + spin_unlock_irqrestore(&split_queue_lock, flags);
> __split_huge_page(page, list);
> ret = 0;
> } else if (IS_ENABLED(CONFIG_DEBUG_VM) && mapcount) {
> - spin_unlock(&split_queue_lock);
> + spin_unlock_irqrestore(&split_queue_lock, flags);
> pr_alert("total_mapcount: %u, page_count(): %u\n",
> mapcount, count);
> if (PageTail(page))
> @@ -3416,7 +3417,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> dump_page(page, "total_mapcount(head) > 0");
> BUG();
> } else {
> - spin_unlock(&split_queue_lock);
> + spin_unlock_irqrestore(&split_queue_lock, flags);
> unfreeze_page(anon_vma, head);
> ret = -EBUSY;
> }
> --
> Kirill A. Shutemov