Re: [PATCH v1] selftest mm/mseal: fix test_seal_mremap_move_dontunmap_anyaddr

On Wed, Aug 7, 2024 at 4:35 PM jeffxu@chromium.org wrote: <snip>

    /* shrink from 4 pages to 2 pages. */

•   ret2 = mremap(ptr, size, 2 * page_size, 0, 0);
  

•   ret2 = sys_mremap(ptr, size, 2 * page_size, 0, 0);
    if (seal) {
  

•           FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
  

•           FAIL_TEST_IF_FALSE(ret2 == (void *) MAP_FAILED);
  

MAP_FAILED is already void *

<snip>

@@ -1449,18 +1457,16 @@ static void test_seal_mremap_move_dontunmap_anyaddr(bool seal) }

    /*

•    * The 0xdeaddead should not have effect on dest addr
  

•    * The 0xdead0000 should not have effect on dest addr
     * when MREMAP_DONTUNMAP is set.
     */
  

•   ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP,
  

•                   0xdeaddead);
  

•   ret2 = sys_mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP,
  

•                   (void *) 0xdead0000);
  

You still didn't explain why this test is actually needed. Why are you testing MREMAP_DONTUNMAP's hint system? This has nothing to do with mseal, you already test the MREMAP_DONTUNMAP and MREMAP_FIXED paths in other tests. You also don't know if 0xdead0000 is a valid page (hexagon for instance seems to support 256KiB and 1MiB pages, so does ppc32, and this is not something that should be hardcoded).

I'm not a fan of just throwing random flags for tests, it should be somewhat logical.