Re: [PATCH] mm, hugepages: fix size in hugetlb mremap() test

[Mike Kravetz]

On 11/23/21 18:19, Yosry Ahmed wrote:
> On Tue, Nov 23, 2021 at 5:08 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
>>
>> On 11/23/21 12:46, Yosry Ahmed wrote:
>>> The hugetlb vma mremap() test mentions in the header comment that it
>>> uses 10MB worth of huge pages, when it actually uses 1GB. This causes
>>> the test to fail on devices with smaller memories.
>>>
>>> Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
>>> ---
>>>  tools/testing/selftests/vm/hugepage-mremap.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> I'll let Mina comment, but I think I know what happened.
> 
> Thanks for taking the time to review this and explain what happened.
> 
>>
>>
>> The original version of the test did indeed use 10MB.  However, the mremap
>> code must 'unshare' and shared pmd mappings before remapping.  Since sharing
>> requires mappings of at least 1GB, the size was changed to make sure unsharing
>> worked.
>>
>> In the end, I believe I suggested adding hugepage-mremap to run_vmtests.sh.
>> The script does not try to configure a GB worth of huge pages.  And, I think
>> it is somewhat unreasonable to suggest users gave a spare GB to run the test.
> 
> Alternatively, we can pass an optional argument to the test that makes it use
> 1GB instead of 10MB. This way, if the test is run with run_vmtests.sh the
> default behavior would be to use 10MB, making sure users do not run out of
> memory. Otherwise, an interested user could run the test without run_vmtest.sh
> and provide the extra argument to make the test use 1GB and make sure that
> unsharing works correctly. Thoughts?
> 

Passing a 'mapping size' argument as you suggest would be best.  That way
run_vmtest.sh can pass in a size such as 10MB, but the test could be used
independently with arbitrary size mappings.

If you have the time to do this, go for it!
-- 
Mike Kravetz