Message-ID: <a87c5768-9bd3-a2f5-7eb5-d2813b958514@oracle.com>
Date:   Tue, 23 Nov 2021 19:42:48 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH] mm, hugepages: fix size in hugetlb mremap() test
Content-Language: en-US
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Shuah Khan <shuah@kernel.org>, linux-mm@kvack.org,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        Mina Almasry <almasrymina@google.com>
References: <20211123204644.3458700-1-yosryahmed@google.com>
 <6ebcffe2-9513-cbea-a206-15ba927416c7@oracle.com>
 <CAJD7tkYZY1g_b9E4ZP3yqHhT36nF57c4bzKRQM-SLftDCYNQ9A@mail.gmail.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <CAJD7tkYZY1g_b9E4ZP3yqHhT36nF57c4bzKRQM-SLftDCYNQ9A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dG1FRVlDL3RJcXFnNGxWWGdHYUhKK245cFJETmN5ZUE5OXByTVlta1ZsSXFm?=
 =?utf-8?B?ZTF4YnhrQ0REd1dkaWpvdVJmYTI4OVpHT1dVT3g5dlFrZU1yOUZ2bERSQXpM?=
 =?utf-8?B?RnhOMEgwK2RCTjRGaHNTNXkvc0NuOFhHNUlUU0pxakJXaUZsRnNvZDZBampO?=
 =?utf-8?B?Zk40eUFzbmpYL3QzR0dSSGRiZTZtS3FlZHoyQnVmS0dQNUJnTDhGUnZRbTZr?=
 =?utf-8?B?cy9JY2t0NC92RmlvT3p4ODFUT2o3N1RXVGg1U05mTkhYV1NON0lxOXBRUWNt?=
 =?utf-8?B?d2JSMlY3UUw0YVJuMFg3UlJGejJhV2pGenlSZDR1WTlGSlVFR0UxaFhQM08w?=
 =?utf-8?B?bG1CdVpsSjJmb0h0eTc0Y015MGREMTUxQkNlYzkxN0RPUC9JOWQzbXY0d3Mw?=
 =?utf-8?B?WjFzb3V5ZXl3ZE1PZXhWUGR6d1UxSitwSTljU1lGWWo3VG56V3d1ODlVZ2Jl?=
 =?utf-8?B?ZCtEZVpvWk00ZTI0TnNPU0dJQmliZ0ppMkZteGx0Wm9aS3Vqcms2a2FWV3h6?=
 =?utf-8?B?UGx2U1RnbHp0QXBlWmlPOFNNdjh0NFNBZ1Vxb0M1cXBrTHpJdGg4Wmh0MWdY?=
 =?utf-8?B?R0tzTTVNMEJlQkd0MWNnck1DTmcvOXhYT2lBK240cWFsSDJlQzRVV25KbzNM?=
 =?utf-8?B?VklrRjF4ZFRXWTU2V1l4S3cyN1UyM0tUbDRsUnNnQUZuS0ZmV1lHYWx2YWRU?=
 =?utf-8?B?cG5iSmI0NkE5VkxycTQwVk8yNzdGbHlIb1YySWxGNmpKY0RLazNEU3dRNWxY?=
 =?utf-8?B?NXJPd0l5aUVvUmNlWGR1b1RrK2FsWmJ4THpWUWp5M2FIYkJRZHoyOW5iK3ZY?=
 =?utf-8?B?L0lSQm9qS3ZWVWZIb3pMdDRSL3Vmd3VsWUFjTGVQYVIyeWloZncrcUk1ekpE?=
 =?utf-8?B?SnEyM2JEd240QWxRdEovWFR0TmlxMys2am01WWJubmpzdThsem85bXFNdTRI?=
 =?utf-8?B?cGk5STJDUGozQkErZ2tscWdDNGtPeDh4SkUra0IzZmxod0ZDVVBKMGdJQy9B?=
 =?utf-8?B?V3pOUVczQ2I4YkdKemg2c2hNS0RaMWxCRDN5amUwaGtOZndYbDQzK09lMnJF?=
 =?utf-8?B?WXExbDBtZ0JjWklPaGNUdFhtL0lZSTk4ZjlOeFFyQ05rcU9hM1gyR2lzN1kz?=
 =?utf-8?B?M1k1Q2FURUZTbzRrQUZEd0dnUlpWS2UxRWV6YWdubUhUU1Y5Z2F4OTNtdDBi?=
 =?utf-8?B?S1RDK2hrdmNzRFc3MXluOGhUbUhFYnZRVHRCSUY3ZTlnTWlWcUJlNk5xNVdB?=
 =?utf-8?B?T0FmcXNueFV2cUZJSEl6TjdpNEp2YkhmM1p1cGJZS3lrMVl6VWxKK1M3NVB5?=
 =?utf-8?B?eVdHZ1kybFRHWUh5T2Z0WWhsVm9ZRDd4ZlRMc3FsOW5nNHUxaXdFQUlvVUtB?=
 =?utf-8?B?dmFvUVBhYkprVmMrbEs1VU5oeFBmMmNEL1BXc09pbVFEUjl2Rks4NVpWcjBh?=
 =?utf-8?B?bWRCYjd3S0QyaTg3K29IdGJNY3NqY0cvZU1iaHFYVFVwelNmUXZMYjhDY1Ro?=
 =?utf-8?B?WEpIRjQ3T2RXcXk4b3FjQ0NMdWMyMUFMWlBrVkhvRC9XOWlGY2ZnVElTaWU0?=
 =?utf-8?B?dStvaTY3WGhBclEzOTd3QkVacVJiZ0hHSWdyejdQTDBNdjBEN0FSRytxS2NS?=
 =?utf-8?B?bTFSM3pyMzFtSDhSV21xeWJCOFd3b1dCUGFyRGloMlFsdmg0MjlucExYNlRN?=
 =?utf-8?B?YkNqMkl2MDZIZ2VEU0xHd25wbFJSSzI2dGRXejNJcmllTXFVbXJKTStQdlM2?=
 =?utf-8?B?dkFCUXRTalh1dit6bXIxdFgrSERQeUFSNjNab28xT212cVYvUWFENTNDUDI1?=
 =?utf-8?B?MlVycEx3OU1MdnA1QVMwbk9Sc0FUVFBlZ3JoZmk0Smx5eFhqZTZLazJEaEth?=
 =?utf-8?B?RWpYOTgrL3lOaHJYMmYvUHJkbmUreHozcFVLcldFdzZaS0x4NEhkMmNhQ2cy?=
 =?utf-8?B?cWkraHR4LzJOWW1wMkRXQlRwTXJHQ0JUU1l2bEMzYlc1STd0dGdlTVB1VnlV?=
 =?utf-8?B?UzJEeXFlb000L0kwekloakgyWWJmSE13TFA4b0dlRExSWFpUZURwVU4rRWZ4?=
 =?utf-8?B?Q0J4bTZkS1duWGtXNzNNNURBU2JuRG0yQVV0UGZhWS8vQjV5UGVyeXA1MEdX?=
 =?utf-8?B?VDhPT1UrMTJ6NU5vWVY0dlM2dkQ2eFpQSHBOdGNoVWU1UU41MCs4cVlRb093?=
 =?utf-8?Q?H2gbRvpSHljwMYYORvguCjo=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 86012930-7f22-4413-e4a0-08d9aefc7e0f
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Nov 2021 03:42:52.1888
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: c3z5TYnyWBdKowGtOy1CwTBJ2Hsgb2Ms1pcS5gMI+NVc7hxzBLKDZvl+gn4CRiELGHcOlybl1roEAGc3REz/dg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2549
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10177 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 malwarescore=0
 mlxlogscore=999 bulkscore=0 mlxscore=0 suspectscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111240019
X-Proofpoint-GUID: jNU8csw4h2sFCZn4K2Jnn6prrtyRYPP1
X-Proofpoint-ORIG-GUID: jNU8csw4h2sFCZn4K2Jnn6prrtyRYPP1
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/23/21 18:19, Yosry Ahmed wrote:
> On Tue, Nov 23, 2021 at 5:08 PM Mike Kravetz <mike.kravetz@oracle.com> 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@google.com>
>>> ---
>>>  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