public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Yadav, Arvind" <arvind.yadav@intel.com>
To: "Sharma, Nishit" <nishit.sharma@intel.com>,
	<igt-dev@lists.freedesktop.org>
Cc: <matthew.brost@intel.com>, <himal.prasad.ghimiray@intel.com>,
	<thomas.hellstrom@linux.intel.com>, <pravalika.gurram@intel.com>
Subject: Re: [PATCH i-g-t v7 5/9] tests/intel/xe_madvise: Add dontneed-after-mmap subtest
Date: Fri, 10 Apr 2026 14:29:43 +0530	[thread overview]
Message-ID: <2ac85b15-8e18-49fd-a33f-e420f6d7b046@intel.com> (raw)
In-Reply-To: <958dbd9c-cbaf-48aa-8375-16e7b8f304d3@intel.com>


On 10-04-2026 13:52, Sharma, Nishit wrote:
>
> On 4/9/2026 12:31 PM, Arvind Yadav wrote:
>> This test verifies that an existing mmap becomes invalid after the BO
>> is marked as purgeable and purged. The test creates a BO, maps it,
>> writes data, then marks it DONTNEED and triggers memory pressure.
>> Accessing the previously valid mapping should now trigger SIGBUS or
>> SIGSEGV, confirming that existing mappings are correctly invalidated
>> when the backing store is purged.
>>
>> v4:
>>    - Added proper resource cleanup before calling igt_skip(). (Nishit)
>>    - Added assertion for xe_bo_map. (Nishit)
>>
>> v6:
>>    - Fix sigsetjmp(jmp, SIGBUS | SIGSEGV) to sigsetjmp(jmp, 1). The
>>      second argument is a plain boolean savemask, not a signal set.
>>
>> v7:
>>    - Move sigtrap/jmp_buf in this patch. (Nishit)
>>
>> Cc: Nishit Sharma <nishit.sharma@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Cc: Pravalika Gurram <pravalika.gurram@intel.com>
>> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
>> ---
>>   tests/intel/xe_madvise.c | 71 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 71 insertions(+)
>>
>> diff --git a/tests/intel/xe_madvise.c b/tests/intel/xe_madvise.c
>> index 619d64b46..2d5acc347 100644
>> --- a/tests/intel/xe_madvise.c
>> +++ b/tests/intel/xe_madvise.c
>> @@ -153,6 +153,13 @@ static bool purgeable_mark_and_verify_purged(int 
>> fd, uint32_t vm, uint64_t addr,
>>       return retained == 0;
>>   }
>>   +static jmp_buf jmp;
>> +
>> +__noreturn static void sigtrap(int sig)
>> +{
>> +    siglongjmp(jmp, sig);
>> +}
>> +
>>   /**
>>    * SUBTEST: purged-mmap-blocked
>>    * Description: After BO is purged, verify mmap() fails with -EINVAL
>> @@ -227,6 +234,64 @@ static void test_dontneed_before_mmap(int fd)
>>       xe_vm_destroy(fd, vm);
>>   }
>>   +/**
>> + * SUBTEST: dontneed-after-mmap
>> + * Description: Mark BO as DONTNEED after mmap, verify SIGBUS on 
>> accessing purged mapping
>> + * Test category: functionality test
>> + */
>> +static void test_dontneed_after_mmap(int fd)
>> +{
>> +    uint32_t bo, vm;
>> +    uint64_t addr = PURGEABLE_ADDR;
>> +    size_t bo_size = PURGEABLE_BO_SIZE;
>> +    void *map;
>> +
>> +    purgeable_setup_simple_bo(fd, &vm, &bo, addr, bo_size, true);
>> +
>> +    map = xe_bo_map(fd, bo, bo_size);
>> +    igt_assert(map != MAP_FAILED);
>> +    memset(map, 0xAB, bo_size);
>> +
>> +    if (!purgeable_mark_and_verify_purged(fd, vm, addr, bo_size)) {
>> +        munmap(map, bo_size);
>> +        gem_close(fd, bo);
>> +        xe_vm_destroy(fd, vm);
>> +        igt_skip("Unable to induce purge on this platform/config");
> Same Nit as in Patch-4/9.
>> +    }
>> +
>> +    /* Access purged mapping - should trigger SIGBUS/SIGSEGV */
>> +    {
>> +        sighandler_t old_sigsegv, old_sigbus;
>> +        char *ptr = (char *)map;
>> +        int sig;
>> +
>> +        old_sigsegv = signal(SIGSEGV, (__sighandler_t)sigtrap);
>> +        old_sigbus = signal(SIGBUS, (__sighandler_t)sigtrap);
>> +
>> +        sig = sigsetjmp(jmp, 1); /* savemask=1: save/restore signal 
>> mask */
>> +        switch (sig) {
>> +        case SIGBUS:
>> +        case SIGSEGV:
>> +            /* Expected - purged mapping access failed */
>> +            break;
>> +        case 0:
>> +            *ptr = 0;
>
> Here if suppose SIGBUS/SIGSEGV is not hit but mapping is invalidated, 
> here you'll get segfault. Remove *ptr = 0 and club case 0: and default 
> and
>
> let it assert as available


If we remove *ptr = 0, we never actually access the purged mapping, so 
SIGBUS is never triggered and the test would always fail on the assert - 
defeating its purpose.
we can say without *ptr = 0, we  never touch the purged mapping at all, 
so no signal would fire and the test would always hit the assert.


Thanks,
Arvind

>
>> +        default:
>> +            igt_assert_f(false,
>> +                     "Access to purged mapping should trigger 
>> SIGBUS, got sig=%d\n",
>> +                     sig);
>> +            break;
>> +        }
>> +
>> +        signal(SIGBUS, old_sigbus);
>> +        signal(SIGSEGV, old_sigsegv);
>> +    }
>> +
>> +    munmap(map, bo_size);
>> +    gem_close(fd, bo);
>> +    xe_vm_destroy(fd, vm);
>> +}
>> +
>>   int igt_main()
>>   {
>>       struct drm_xe_engine_class_instance *hwe;
>> @@ -251,6 +316,12 @@ int igt_main()
>>               break;
>>           }
>>   +    igt_subtest("dontneed-after-mmap")
>> +        xe_for_each_engine(fd, hwe) {
>> +            test_dontneed_after_mmap(fd);
>> +            break;
>> +        }
>> +
>>       igt_fixture() {
>>           xe_device_put(fd);
>>           drm_close_driver(fd);
>
> With above changes LGTM:
>
> Reviewed-by: Nishit Sharma <nishit.sharma@intel.com>
>

  reply	other threads:[~2026-04-10  9:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09  7:01 [PATCH i-g-t v7 0/9] tests/xe: Add purgeable memory madvise tests for system allocator Arvind Yadav
2026-04-09  7:01 ` [PATCH i-g-t v7 1/9] drm-uapi/xe_drm: Sync with Add UAPI support for purgeable buffer objects Arvind Yadav
2026-04-09 15:42   ` Kamil Konieczny
2026-04-10  5:35     ` Yadav, Arvind
2026-04-09  7:01 ` [PATCH i-g-t v7 2/9] lib/xe: Add purgeable memory ioctl support Arvind Yadav
2026-04-09  7:01 ` [PATCH i-g-t v7 3/9] tests/intel/xe_madvise: Add dontneed-before-mmap subtest Arvind Yadav
2026-04-10  7:10   ` Sharma, Nishit
2026-04-09  7:01 ` [PATCH i-g-t v7 4/9] tests/intel/xe_madvise: Add purged-mmap-blocked subtest Arvind Yadav
2026-04-10  8:06   ` Sharma, Nishit
2026-04-09  7:01 ` [PATCH i-g-t v7 5/9] tests/intel/xe_madvise: Add dontneed-after-mmap subtest Arvind Yadav
2026-04-10  8:22   ` Sharma, Nishit
2026-04-10  8:59     ` Yadav, Arvind [this message]
2026-04-09  7:01 ` [PATCH i-g-t v7 6/9] tests/intel/xe_madvise: Add dontneed-before-exec subtest Arvind Yadav
2026-04-10  8:32   ` Sharma, Nishit
2026-04-09  7:01 ` [PATCH i-g-t v7 7/9] tests/intel/xe_madvise: Add dontneed-after-exec subtest Arvind Yadav
2026-04-09  7:01 ` [PATCH i-g-t v7 8/9] tests/intel/xe_madvise: Add per-vma-tracking subtest Arvind Yadav
2026-04-10  8:41   ` Sharma, Nishit
2026-04-09  7:01 ` [PATCH i-g-t v7 9/9] tests/intel/xe_madvise: Add per-vma-protection subtest Arvind Yadav
2026-04-10  1:08 ` ✗ Fi.CI.BUILD: failure for tests/xe: Add purgeable memory madvise tests for system allocator (rev7) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ac85b15-8e18-49fd-a33f-e420f6d7b046@intel.com \
    --to=arvind.yadav@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=nishit.sharma@intel.com \
    --cc=pravalika.gurram@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox