Re: Hangs in libhugetlbfs tests in -next

Re: Hangs in libhugetlbfs tests in -next

[Steve Capper]

On 28 January 2015 at 15:24, Mark Brown <broonie@kernel.org> wrote:
> For at least the past couple of days tests of libhugetlbfs have been
> hanging on mustang in the mlock test running ARMv8 defconfig with both
> 32 bit and 64 bit userspace - after the mprotect test (the one before
> it) we get no console output for several hours so it appears that the
> test has deadlocked.  The tail of the log is:
>
> | find_path (2M: 64):   PASS
> | unlinked_fd (2M: 64): PASS
> | readback (2M: 64):    PASS
> | truncate (2M: 64):    PASS
> | shared (2M: 64):      PASS
> | mprotect (2M: 64):    PASS
>
> and then a timeout.
>
> This may be a generic bug but my ability to run these tests on other
> platforms with huge page support using the infrastructure I'm using is
> limited so I've not been able to test elsewhere and confirm.

I've bisected this problem down to commit:
f2c0507 mm: remove remaining references to NUMA hinting bits and helpers

Essentially the mlock unit test from libhugetlbfs then fails with
hugetlbf_fault being called in an infinite loop.

After this commit is applied we get the following definition of:
static inline int is_swap_pte(pte_t pte)
{
        return !pte_none(pte);
}

rather than:
static inline int is_swap_pte(pte_t pte)
{
        return !pte_none(pte) && !pte_present_nonuma(pte);
}

(note here that pte_present_nonuma == pte_present on arm64)

I think this is a typo in the patch, as entries should be checked for
present before being determined to be swap or file?

Cheers,
-- 
Steve

Re: Hangs in libhugetlbfs tests in -next

[Catalin Marinas]

On Thu, Jan 29, 2015 at 04:02:27PM +0000, Steve Capper wrote:
> On 28 January 2015 at 15:24, Mark Brown <broonie@kernel.org> wrote:
> > For at least the past couple of days tests of libhugetlbfs have been
> > hanging on mustang in the mlock test running ARMv8 defconfig with both
> > 32 bit and 64 bit userspace - after the mprotect test (the one before
> > it) we get no console output for several hours so it appears that the
> > test has deadlocked.  The tail of the log is:
> >
> > | find_path (2M: 64):   PASS
> > | unlinked_fd (2M: 64): PASS
> > | readback (2M: 64):    PASS
> > | truncate (2M: 64):    PASS
> > | shared (2M: 64):      PASS
> > | mprotect (2M: 64):    PASS
> >
> > and then a timeout.
> >
> > This may be a generic bug but my ability to run these tests on other
> > platforms with huge page support using the infrastructure I'm using is
> > limited so I've not been able to test elsewhere and confirm.
> 
> I've bisected this problem down to commit:
> f2c0507 mm: remove remaining references to NUMA hinting bits and helpers
> 
> Essentially the mlock unit test from libhugetlbfs then fails with
> hugetlbf_fault being called in an infinite loop.
> 
> After this commit is applied we get the following definition of:
> static inline int is_swap_pte(pte_t pte)
> {
>         return !pte_none(pte);
> }
> 
> rather than:
> static inline int is_swap_pte(pte_t pte)
> {
>         return !pte_none(pte) && !pte_present_nonuma(pte);
> }
> 
> (note here that pte_present_nonuma == pte_present on arm64)
> 
> I think this is a typo in the patch, as entries should be checked for
> present before being determined to be swap or file?

pte_file() check is removed in commit 7823d501c417fc but I can't tell
exactly why, I haven't followed the patch series. But Mel's commit
removes pte_present_nonuma() entirely, I think it should have been
replaced with pte_present().

-- 
Catalin

[PATCH] mm: remove remaining references to NUMA hinting bits and helpers -fix

[Mel Gorman]

The patch "mm: remove remaining references to NUMA hinting bits and helpers"
was meant to remove NUMA hinting bits and helpers but broke is_swap_pte.
>From the original bug report

	For at least the past couple of days tests of libhugetlbfs have
	been hanging on mustang in the mlock test running ARMv8 defconfig
	with both 32 bit and 64 bit userspace - after the mprotect test
	(the one before it) we get no console output for several hours so
	it appears that the test has deadlocked.  The tail of the log is:

	| find_path (2M: 64):   PASS
	| unlinked_fd (2M: 64): PASS
	| readback (2M: 64):    PASS
	| truncate (2M: 64):    PASS
	| shared (2M: 64):      PASS
	| mprotect (2M: 64):    PASS

This patch should address the problem and is a fix to the mmotm patch
mm-remove-remaining-references-to-numa-hinting-bits-and-helpers.patch

Reported-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 include/linux/swapops.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index 73ca28070a92..cedf3d3c373f 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -54,7 +54,7 @@ static inline pgoff_t swp_offset(swp_entry_t entry)
 /* check whether a pte points to a swap entry */
 static inline int is_swap_pte(pte_t pte)
 {
-	return !pte_none(pte);
+	return !pte_none(pte) && !pte_present(pte);
 }
 #endif
 

Re: [PATCH] mm: remove remaining references to NUMA hinting bits and helpers -fix

[Steve Capper]

On Fri, Jan 30, 2015 at 10:04:06AM +0000, Mel Gorman wrote:
> The patch "mm: remove remaining references to NUMA hinting bits and helpers"
> was meant to remove NUMA hinting bits and helpers but broke is_swap_pte.
> From the original bug report
> 
> 	For at least the past couple of days tests of libhugetlbfs have
> 	been hanging on mustang in the mlock test running ARMv8 defconfig
> 	with both 32 bit and 64 bit userspace - after the mprotect test
> 	(the one before it) we get no console output for several hours so
> 	it appears that the test has deadlocked.  The tail of the log is:
> 
> 	| find_path (2M: 64):   PASS
> 	| unlinked_fd (2M: 64): PASS
> 	| readback (2M: 64):    PASS
> 	| truncate (2M: 64):    PASS
> 	| shared (2M: 64):      PASS
> 	| mprotect (2M: 64):    PASS
> 
> This patch should address the problem and is a fix to the mmotm patch
> mm-remove-remaining-references-to-numa-hinting-bits-and-helpers.patch
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Mel Gorman <mgorman@suse.de>

Acked-by: Steve Capper <steve.capper@linaro.org>

> ---
>  include/linux/swapops.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index 73ca28070a92..cedf3d3c373f 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -54,7 +54,7 @@ static inline pgoff_t swp_offset(swp_entry_t entry)
>  /* check whether a pte points to a swap entry */
>  static inline int is_swap_pte(pte_t pte)
>  {
> -	return !pte_none(pte);
> +	return !pte_none(pte) && !pte_present(pte);
>  }
>  #endif
>