vunmap() on large regions may trigger soft lockup warnings

Andrew,

Dietmar Hahn reported an issue where calling vunmap() on a large (50 GB) region would trigger soft lockup warnings.

The following patch would resolve this (by adding a cond_resched() call to vunmap_pmd_range()). Almost calls of vunmap(), unmap_kernel_range() are from process context (as far as I could tell) except for an ACPI driver (drivers/acpi/apei/ghes.c) calls unmap_kernel_range_noflush() from an interrupt and NMI contexts.

Can you advise on a preferred solution?

For example, an unmap_kernel_page() function (callable from atomic context) could be provided since the GHES driver only maps/unmaps a single page.

8<-------------------------
mm/vmalloc: avoid soft lockup warnings when vunmap()'ing large ranges

From: David Vrabel <[email protected]>

If vunmap() is used to unmap a large (e.g., 50 GB) region, it may take sufficiently long that it triggers soft lockup warnings.

Add a cond_resched() into vunmap_pmd_range() so the calling task may be resheduled after unmapping each PMD entry. This is how zap_pmd_range() fixes the same problem for userspace mappings.

...

Andrew,

Dietmar Hahn reported an issue where calling vunmap() on a large (50 GB) region would trigger soft lockup warnings.

The following patch would resolve this (by adding a cond_resched() call to vunmap_pmd_range()). Almost calls of vunmap(), unmap_kernel_range() are from process context (as far as I could tell) except for an ACPI driver (drivers/acpi/apei/ghes.c) calls unmap_kernel_range_noflush() from an interrupt and NMI contexts.

Can you advise on a preferred solution?

For example, an unmap_kernel_page() function (callable from atomic context) could be provided since the GHES driver only maps/unmaps a single page.

8<-------------------------
mm/vmalloc: avoid soft lockup warnings when vunmap()'ing large ranges

From: David Vrabel <[email protected]>

If vunmap() is used to unmap a large (e.g., 50 GB) region, it may take sufficiently long that it triggers soft lockup warnings.

Add a cond_resched() into vunmap_pmd_range() so the calling task may be resheduled after unmapping each PMD entry. This is how zap_pmd_range() fixes the same problem for userspace mappings.

...

Quoted 34 lines · On Wed, 11 Dec 2013 16:58:19 +0000 David Vrabel <[email protected]> wrote:

Andrew,

Dietmar Hahn reported an issue where calling vunmap() on a large (50 GB) region would trigger soft lockup warnings.

The following patch would resolve this (by adding a cond_resched() call to vunmap_pmd_range()). Almost calls of vunmap(), unmap_kernel_range() are from process context (as far as I could tell) except for an ACPI driver (drivers/acpi/apei/ghes.c) calls unmap_kernel_range_noflush() from an interrupt and NMI contexts.

Can you advise on a preferred solution?

For example, an unmap_kernel_page() function (callable from atomic context) could be provided since the GHES driver only maps/unmaps a single page.

8<-------------------------
mm/vmalloc: avoid soft lockup warnings when vunmap()'ing large ranges

From: David Vrabel <[email protected]>

If vunmap() is used to unmap a large (e.g., 50 GB) region, it may take sufficiently long that it triggers soft lockup warnings.

Add a cond_resched() into vunmap_pmd_range() so the calling task may be resheduled after unmapping each PMD entry. This is how zap_pmd_range() fixes the same problem for userspace mappings.

...

mm/vmalloc.c static void vunmap_pmd_range(pud_t *pud, unsigned long · line 81

Quoted 7 lines

mm/vmalloc.c +1 -0

@@ -75,6 +75,7 @@ static void vunmap_pmd_range(pud_t *pud, unsigned long
 addr, unsigned long end)
 		if (pmd_none_or_clear_bad(pmd))
 			continue;
 		vunmap_pte_range(pmd, addr, next);
+		cond_resched();
 	} while (pmd++, addr = next, addr != end);
 }

Well that's ugly.

We could redo unmap_kernel_range() so it takes an `atomic' flag then loops around unmapping N MB at a time, doing

if (!atomic)
cond_resched()

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

Quoted 34 lines · On Wed, 11 Dec 2013 16:58:19 +0000 David Vrabel <[email protected]> wrote:

Andrew,

Dietmar Hahn reported an issue where calling vunmap() on a large (50 GB) region would trigger soft lockup warnings.

The following patch would resolve this (by adding a cond_resched() call to vunmap_pmd_range()). Almost calls of vunmap(), unmap_kernel_range() are from process context (as far as I could tell) except for an ACPI driver (drivers/acpi/apei/ghes.c) calls unmap_kernel_range_noflush() from an interrupt and NMI contexts.

Can you advise on a preferred solution?

For example, an unmap_kernel_page() function (callable from atomic context) could be provided since the GHES driver only maps/unmaps a single page.

8<-------------------------
mm/vmalloc: avoid soft lockup warnings when vunmap()'ing large ranges

From: David Vrabel <[email protected]>

If vunmap() is used to unmap a large (e.g., 50 GB) region, it may take sufficiently long that it triggers soft lockup warnings.

Add a cond_resched() into vunmap_pmd_range() so the calling task may be resheduled after unmapping each PMD entry. This is how zap_pmd_range() fixes the same problem for userspace mappings.

...

mm/vmalloc.c static void vunmap_pmd_range(pud_t *pud, unsigned long · line 81

Quoted 7 lines

mm/vmalloc.c +1 -0

@@ -75,6 +75,7 @@ static void vunmap_pmd_range(pud_t *pud, unsigned long
 addr, unsigned long end)
 		if (pmd_none_or_clear_bad(pmd))
 			continue;
 		vunmap_pte_range(pmd, addr, next);
+		cond_resched();
 	} while (pmd++, addr = next, addr != end);
 }

Well that's ugly.

We could redo unmap_kernel_range() so it takes an `atomic' flag then loops around unmapping N MB at a time, doing

if (!atomic)
cond_resched()

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Quoted 19 lines · On Thu, 12 Dec 2013 12:50:47 +0000 David Vrabel <[email protected]> wrote:

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Quoted 3 lines

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Is ghes.c the only problem source here? If so then a suitable solution would be to declare that driver hopelessly busted and proceed as if it didn't exist :(

Quoted 19 lines · On Thu, 12 Dec 2013 12:50:47 +0000 David Vrabel <[email protected]> wrote:

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Quoted 3 lines

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Is ghes.c the only problem source here? If so then a suitable solution would be to declare that driver hopelessly busted and proceed as if it didn't exist :(

Quoted 21 lines · Andrew Morton

Quoted 19 lines · On Thu, 12 Dec 2013 12:50:47 +0000 David Vrabel <[email protected]> wrote:

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Quoted 7 lines

Quoted 3 lines

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Is ghes.c the only problem source here? If so then a suitable solution would be to declare that driver hopelessly busted and proceed as if it didn't exist :(

All the other callers do so from non-atomic context. ghes.c is the only broken caller.

Shall I resend or are you happy to take the patch off the first email in this thread?

Quoted 21 lines · Andrew Morton

Quoted 19 lines · On Thu, 12 Dec 2013 12:50:47 +0000 David Vrabel <[email protected]> wrote:

Quoted 7 lines

each time. But that would require difficult tuning of N.

I suppose we could just do

	if (!in_interrupt())
		cond_resched();

Quoted 6 lines

in vunmap_pmd_range(), but that's pretty specific to ghes.c and doesn't permit unmap-inside-spinlock.

So I can't immediately think of a suitable fix apart from adding a new unmap_kernel_range_atomic(). Then add a `bool atomic' arg to vunmap_page_range() and pass that all the way down.

That would work for the unmap, but looking at the GHES driver some more and it looks like it's call to ioremap_page_range() is already unsafe -- it may need to allocate a new PTE page with a non-atomic alloc in pte_alloc_one_kernel().

Quoted 7 lines

Quoted 3 lines

Perhaps what's needed here is a pair of ioremap_page_atomic() and iounmap_page_atomic() calls? With some prep function to sure the PTE pages (etc.) are preallocated.

Is ghes.c the only problem source here? If so then a suitable solution would be to declare that driver hopelessly busted and proceed as if it didn't exist :(

All the other callers do so from non-atomic context. ghes.c is the only broken caller.

Shall I resend or are you happy to take the patch off the first email in this thread?