All of lore.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.