* [igt-dev] [PATCH i-g-t] lib: Incrementally mlock()
@ 2019-02-27 16:29 Chris Wilson
2019-02-27 17:15 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2019-02-27 16:29 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
As we already have the previous portion of the mmap mlocked, we only
need to mlock() the fresh portion for testing available memory.
v2: Fixup the uint64_t pointer arithmetric and only use a single mmap to
avoid subsequent mlock fail (for reasons unknown, but bet on mm/).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Caz Yokoyama <Caz.Yokoyama@intel.com>
---
lib/igt_aux.h | 2 +-
lib/intel_os.c | 40 +++++++++++++++++++++------------------
tests/eviction_common.c | 17 +++++++++--------
tests/i915/i915_suspend.c | 17 +++--------------
4 files changed, 35 insertions(+), 41 deletions(-)
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index fb02c026a..09b6246bf 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -194,7 +194,7 @@ void intel_purge_vm_caches(int fd);
uint64_t intel_get_avail_ram_mb(void);
uint64_t intel_get_total_ram_mb(void);
uint64_t intel_get_total_swap_mb(void);
-size_t intel_get_total_pinnable_mem(void);
+void *intel_get_total_pinnable_mem(size_t *pinned);
int __intel_check_memory(uint64_t count, uint64_t size, unsigned mode,
uint64_t *out_required, uint64_t *out_total);
diff --git a/lib/intel_os.c b/lib/intel_os.c
index e1e31e230..dd93bea1a 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -227,11 +227,9 @@ intel_get_total_swap_mb(void)
*
* Returns: Amount of memory that can be safely pinned, in bytes.
*/
-size_t
-intel_get_total_pinnable_mem(void)
+void *intel_get_total_pinnable_mem(size_t *total)
{
uint64_t *can_mlock, pin, avail;
- size_t ret;
pin = (intel_get_total_ram_mb() + 1) << 20;
avail = (intel_get_avail_ram_mb() + 1) << 20;
@@ -245,34 +243,40 @@ intel_get_total_pinnable_mem(void)
*/
*can_mlock = (avail >> 1) + (avail >> 2);
if (mlock(can_mlock, *can_mlock)) {
- *can_mlock = 0;
- goto out;
+ munmap(can_mlock, pin);
+ return MAP_FAILED;
}
for (uint64_t inc = 1024 << 20; inc >= 4 << 10; inc >>= 2) {
- igt_debug("Testing mlock %'"PRIu64" B (%'"PRIu64" MiB)\n",
- *can_mlock, *can_mlock >> 20);
+ uint64_t locked = *can_mlock;
+
+ igt_debug("Testing mlock %'"PRIu64"B (%'"PRIu64"MiB) + %'"PRIu64"B\n",
+ locked, locked >> 20, inc);
igt_fork(child, 1) {
- for (uint64_t bytes = *can_mlock;
- bytes <= pin;
- bytes += inc) {
- if (mlock(can_mlock, bytes))
+ uint64_t bytes = *can_mlock;
+
+ while (bytes <= pin) {
+ if (mlock((void *)can_mlock + bytes, inc))
break;
- *can_mlock = bytes;
+ *can_mlock = bytes += inc;
__sync_synchronize();
}
}
__igt_waitchildren();
- igt_assert(!mlock(can_mlock, *can_mlock));
- }
-out:
- ret = *can_mlock;
- munmap(can_mlock, pin);
+ if (*can_mlock > locked + inc) { /* Weird bit of mm/ lore */
+ *can_mlock -= inc;
+ igt_debug("Claiming mlock %'"PRIu64"B (%'"PRIu64"MiB)\n",
+ *can_mlock, *can_mlock >> 20);
+ igt_assert(!mlock((void *)can_mlock + locked,
+ *can_mlock - locked));
+ }
+ }
- return ret;
+ *total = pin;
+ return can_mlock;
}
static uint64_t vfs_file_max(void)
diff --git a/tests/eviction_common.c b/tests/eviction_common.c
index 321772ba7..a3b9e4167 100644
--- a/tests/eviction_common.c
+++ b/tests/eviction_common.c
@@ -133,23 +133,24 @@ static void mlocked_evictions(int fd, struct igt_eviction_test_ops *ops,
uint64_t surface_size,
uint64_t surface_count)
{
+ uint64_t sz, pin, total;
void *mem;
- uint64_t sz, pin_total;
intel_require_memory(surface_count, surface_size, CHECK_RAM);
- sz = surface_size*surface_count;
- pin_total = intel_get_total_pinnable_mem();
- igt_require(pin_total > sz);
-
- mem = mmap(NULL, pin_total, PROT_READ, MAP_SHARED | MAP_ANON, -1, 0);
+ mem = intel_get_total_pinnable_mem(&total);
igt_assert(mem != MAP_FAILED);
+ pin = *(uint64_t *)mem;
+ igt_assert(!munlock(mem, pin));
+
+ sz = surface_size * surface_count;
+ igt_require(pin > sz);
igt_fork(child, 1) {
uint32_t *bo;
uint64_t n;
int ret;
- size_t lock = pin_total - sz;
+ size_t lock = pin - sz;
bo = malloc(surface_count * sizeof(*bo));
igt_assert(bo);
@@ -186,7 +187,7 @@ static void mlocked_evictions(int fd, struct igt_eviction_test_ops *ops,
}
igt_waitchildren();
- munmap(mem, pin_total);
+ munmap(mem, total);
}
static int swapping_evictions(int fd, struct igt_eviction_test_ops *ops,
diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
index 84cb3b490..cd7cf9675 100644
--- a/tests/i915/i915_suspend.c
+++ b/tests/i915/i915_suspend.c
@@ -163,25 +163,14 @@ test_sysfs_reader(bool hibernate)
static void
test_shrink(int fd, unsigned int mode)
{
- void *mem;
size_t size;
+ void *mem;
gem_quiescent_gpu(fd);
intel_purge_vm_caches(fd);
- size = intel_get_total_pinnable_mem();
- igt_require(size > 64 << 20);
- size -= 64 << 20;
-
- mem = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_ANON, -1, 0);
-
- intel_purge_vm_caches(fd);
-
- igt_debug("Locking %'zu B (%'zu MiB)\n",
- size, size >> 20);
- igt_assert(!mlock(mem, size));
- igt_info("Locked %'zu B (%'zu MiB)\n",
- size, size >> 20);
+ mem = intel_get_total_pinnable_mem(&size);
+ igt_assert(mem != MAP_FAILED);
intel_purge_vm_caches(fd);
igt_system_suspend_autoresume(mode, SUSPEND_TEST_NONE);
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for lib: Incrementally mlock()
2019-02-27 16:29 [igt-dev] [PATCH i-g-t] lib: Incrementally mlock() Chris Wilson
@ 2019-02-27 17:15 ` Patchwork
2019-02-27 17:34 ` [igt-dev] [PATCH i-g-t] " Caz Yokoyama
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-02-27 17:15 UTC (permalink / raw)
To: igt-dev
== Series Details ==
Series: lib: Incrementally mlock()
URL : https://patchwork.freedesktop.org/series/57302/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5666 -> IGTPW_2532
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_2532 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_2532, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/57302/revisions/1/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_2532:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_gem:
- fi-skl-gvtdvm: PASS -> DMESG-WARN
Known issues
------------
Here are the changes found in IGTPW_2532 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_contexts:
- fi-icl-u2: NOTRUN -> DMESG-FAIL [fdo#108569]
* igt@kms_busy@basic-flip-a:
- fi-gdg-551: PASS -> FAIL [fdo#103182]
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: PASS -> WARN [fdo#109380]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
- fi-kbl-7567u: PASS -> SKIP [fdo#109271] +33
* igt@kms_psr@primary_mmap_gtt:
- fi-blb-e6850: NOTRUN -> SKIP [fdo#109271] +27
* igt@prime_vgem@basic-fence-flip:
- fi-ilk-650: PASS -> FAIL [fdo#104008]
* igt@runner@aborted:
- fi-skl-gvtdvm: NOTRUN -> FAIL [fdo#104108]
#### Possible fixes ####
* igt@i915_selftest@live_execlists:
- fi-apl-guc: INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS
* igt@i915_selftest@live_requests:
- fi-icl-u2: INCOMPLETE [fdo#109644] -> PASS
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@prime_vgem@basic-fence-flip:
- fi-gdg-551: FAIL [fdo#103182] -> PASS
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
[fdo#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
[fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
Participating hosts (44 -> 37)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510 fi-icl-y
Build changes
-------------
* IGT: IGT_4860 -> IGTPW_2532
CI_DRM_5666: 358ab8acaabef3cef2a7ce9e5dd7c4196a0c30fc @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2532: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2532/
IGT_4860: b79007f9c575a538a63ce9301a890ed9e1a45f35 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2532/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] lib: Incrementally mlock()
2019-02-27 16:29 [igt-dev] [PATCH i-g-t] lib: Incrementally mlock() Chris Wilson
2019-02-27 17:15 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2019-02-27 17:34 ` Caz Yokoyama
2019-02-27 21:15 ` Chris Wilson
2019-02-28 18:56 ` Caz Yokoyama
2019-03-01 6:27 ` [igt-dev] [Intel-gfx] " Ashutosh Dixit
3 siblings, 1 reply; 6+ messages in thread
From: Caz Yokoyama @ 2019-02-27 17:34 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
inline.
v2 patches fixes
- address calculation bug
- not killed
However, the test does not runs faster.
-caz
On Wed, 2019-02-27 at 16:29 +0000, Chris Wilson wrote:
> As we already have the previous portion of the mmap mlocked, we only
> need to mlock() the fresh portion for testing available memory.
>
> v2: Fixup the uint64_t pointer arithmetric and only use a single mmap
> to
> avoid subsequent mlock fail (for reasons unknown, but bet on mm/).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Caz Yokoyama <Caz.Yokoyama@intel.com>
> ---
> lib/igt_aux.h | 2 +-
> lib/intel_os.c | 40 +++++++++++++++++++++--------------
> ----
> tests/eviction_common.c | 17 +++++++++--------
> tests/i915/i915_suspend.c | 17 +++--------------
> 4 files changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index fb02c026a..09b6246bf 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -194,7 +194,7 @@ void intel_purge_vm_caches(int fd);
> uint64_t intel_get_avail_ram_mb(void);
> uint64_t intel_get_total_ram_mb(void);
> uint64_t intel_get_total_swap_mb(void);
> -size_t intel_get_total_pinnable_mem(void);
> +void *intel_get_total_pinnable_mem(size_t *pinned);
>
> int __intel_check_memory(uint64_t count, uint64_t size, unsigned
> mode,
> uint64_t *out_required, uint64_t *out_total);
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index e1e31e230..dd93bea1a 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -227,11 +227,9 @@ intel_get_total_swap_mb(void)
> *
> * Returns: Amount of memory that can be safely pinned, in bytes.
> */
> -size_t
> -intel_get_total_pinnable_mem(void)
> +void *intel_get_total_pinnable_mem(size_t *total)
> {
> uint64_t *can_mlock, pin, avail;
> - size_t ret;
>
> pin = (intel_get_total_ram_mb() + 1) << 20;
> avail = (intel_get_avail_ram_mb() + 1) << 20;
> @@ -245,34 +243,40 @@ intel_get_total_pinnable_mem(void)
> */
> *can_mlock = (avail >> 1) + (avail >> 2);
> if (mlock(can_mlock, *can_mlock)) {
> - *can_mlock = 0;
> - goto out;
> + munmap(can_mlock, pin);
> + return MAP_FAILED;
> }
>
> for (uint64_t inc = 1024 << 20; inc >= 4 << 10; inc >>= 2) {
> - igt_debug("Testing mlock %'"PRIu64" B (%'"PRIu64"
> MiB)\n",
> - *can_mlock, *can_mlock >> 20);
> + uint64_t locked = *can_mlock;
> +
> + igt_debug("Testing mlock %'"PRIu64"B (%'"PRIu64"MiB) +
> %'"PRIu64"B\n",
> + locked, locked >> 20, inc);
>
> igt_fork(child, 1) {
> - for (uint64_t bytes = *can_mlock;
> - bytes <= pin;
> - bytes += inc) {
> - if (mlock(can_mlock, bytes))
> + uint64_t bytes = *can_mlock;
> +
> + while (bytes <= pin) {
> + if (mlock((void *)can_mlock + bytes,
> inc))
> break;
>
> - *can_mlock = bytes;
> + *can_mlock = bytes += inc;
> __sync_synchronize();
> }
> }
> __igt_waitchildren();
> - igt_assert(!mlock(can_mlock, *can_mlock));
> - }
>
> -out:
> - ret = *can_mlock;
> - munmap(can_mlock, pin);
> + if (*can_mlock > locked + inc) { /* Weird bit of mm/
> lore */
> + *can_mlock -= inc;
>
Are you able to explain this? Is this for when child is killed during
updating *can_mlock?
If the condition is not met, parent does not mlock. Is this OK?
> + igt_debug("Claiming mlock %'"PRIu64"B
> (%'"PRIu64"MiB)\n",
> + *can_mlock, *can_mlock >> 20);
> + igt_assert(!mlock((void *)can_mlock + locked,
>
> + *can_mlock - locked));
> + }
> + }
>
> - return ret;
> + *total = pin;
>
*total = *can_mlock?
Also you don't unmap. I mean map and unmap are not paired in the same
function. Not elegant. But c'est la vie.
-caz
> + return can_mlock;
> }
>
> static uint64_t vfs_file_max(void)
> diff --git a/tests/eviction_common.c b/tests/eviction_common.c
> index 321772ba7..a3b9e4167 100644
> --- a/tests/eviction_common.c
> +++ b/tests/eviction_common.c
> @@ -133,23 +133,24 @@ static void mlocked_evictions(int fd, struct
> igt_eviction_test_ops *ops,
> uint64_t surface_size,
> uint64_t surface_count)
> {
> + uint64_t sz, pin, total;
> void *mem;
> - uint64_t sz, pin_total;
>
> intel_require_memory(surface_count, surface_size, CHECK_RAM);
>
> - sz = surface_size*surface_count;
> - pin_total = intel_get_total_pinnable_mem();
> - igt_require(pin_total > sz);
> -
> - mem = mmap(NULL, pin_total, PROT_READ, MAP_SHARED | MAP_ANON,
> -1, 0);
> + mem = intel_get_total_pinnable_mem(&total);
> igt_assert(mem != MAP_FAILED);
> + pin = *(uint64_t *)mem;
> + igt_assert(!munlock(mem, pin));
> +
> + sz = surface_size * surface_count;
> + igt_require(pin > sz);
>
> igt_fork(child, 1) {
> uint32_t *bo;
> uint64_t n;
> int ret;
> - size_t lock = pin_total - sz;
> + size_t lock = pin - sz;
>
> bo = malloc(surface_count * sizeof(*bo));
> igt_assert(bo);
> @@ -186,7 +187,7 @@ static void mlocked_evictions(int fd, struct
> igt_eviction_test_ops *ops,
> }
> igt_waitchildren();
>
> - munmap(mem, pin_total);
> + munmap(mem, total);
> }
>
> static int swapping_evictions(int fd, struct igt_eviction_test_ops
> *ops,
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> index 84cb3b490..cd7cf9675 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -163,25 +163,14 @@ test_sysfs_reader(bool hibernate)
> static void
> test_shrink(int fd, unsigned int mode)
> {
> - void *mem;
> size_t size;
> + void *mem;
>
> gem_quiescent_gpu(fd);
> intel_purge_vm_caches(fd);
>
> - size = intel_get_total_pinnable_mem();
> - igt_require(size > 64 << 20);
> - size -= 64 << 20;
> -
> - mem = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_ANON, -1,
> 0);
> -
> - intel_purge_vm_caches(fd);
> -
> - igt_debug("Locking %'zu B (%'zu MiB)\n",
> - size, size >> 20);
> - igt_assert(!mlock(mem, size));
> - igt_info("Locked %'zu B (%'zu MiB)\n",
> - size, size >> 20);
> + mem = intel_get_total_pinnable_mem(&size);
> + igt_assert(mem != MAP_FAILED);
>
> intel_purge_vm_caches(fd);
> igt_system_suspend_autoresume(mode, SUSPEND_TEST_NONE);
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] lib: Incrementally mlock()
2019-02-27 17:34 ` [igt-dev] [PATCH i-g-t] " Caz Yokoyama
@ 2019-02-27 21:15 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2019-02-27 21:15 UTC (permalink / raw)
To: Caz Yokoyama, intel-gfx; +Cc: igt-dev
Quoting Caz Yokoyama (2019-02-27 17:34:57)
> inline.
>
> v2 patches fixes
> - address calculation bug
> - not killed
> However, the test does not runs faster.
Fair enough, I expected some improvement from the incremental and
avoiding the second mlock + populate of a large region, but if it's not
significant, then let's just make oomkiller faster ;)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] lib: Incrementally mlock()
2019-02-27 16:29 [igt-dev] [PATCH i-g-t] lib: Incrementally mlock() Chris Wilson
2019-02-27 17:15 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2019-02-27 17:34 ` [igt-dev] [PATCH i-g-t] " Caz Yokoyama
@ 2019-02-28 18:56 ` Caz Yokoyama
2019-03-01 6:27 ` [igt-dev] [Intel-gfx] " Ashutosh Dixit
3 siblings, 0 replies; 6+ messages in thread
From: Caz Yokoyama @ 2019-02-28 18:56 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
Reviewed-by: Caz Yokoyama <caz.yokoyama@intel.com>
Correction of my test report: The patched test runs less than 7 sec on
my nuc. 10 times faster.
-caz
On Wed, 2019-02-27 at 16:29 +0000, Chris Wilson wrote:
> As we already have the previous portion of the mmap mlocked, we only
> need to mlock() the fresh portion for testing available memory.
>
> v2: Fixup the uint64_t pointer arithmetric and only use a single mmap
> to
> avoid subsequent mlock fail (for reasons unknown, but bet on mm/).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Caz Yokoyama <Caz.Yokoyama@intel.com>
> ---
> lib/igt_aux.h | 2 +-
> lib/intel_os.c | 40 +++++++++++++++++++++--------------
> ----
> tests/eviction_common.c | 17 +++++++++--------
> tests/i915/i915_suspend.c | 17 +++--------------
> 4 files changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index fb02c026a..09b6246bf 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -194,7 +194,7 @@ void intel_purge_vm_caches(int fd);
> uint64_t intel_get_avail_ram_mb(void);
> uint64_t intel_get_total_ram_mb(void);
> uint64_t intel_get_total_swap_mb(void);
> -size_t intel_get_total_pinnable_mem(void);
> +void *intel_get_total_pinnable_mem(size_t *pinned);
>
> int __intel_check_memory(uint64_t count, uint64_t size, unsigned
> mode,
> uint64_t *out_required, uint64_t *out_total);
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index e1e31e230..dd93bea1a 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -227,11 +227,9 @@ intel_get_total_swap_mb(void)
> *
> * Returns: Amount of memory that can be safely pinned, in bytes.
> */
> -size_t
> -intel_get_total_pinnable_mem(void)
> +void *intel_get_total_pinnable_mem(size_t *total)
> {
> uint64_t *can_mlock, pin, avail;
> - size_t ret;
>
> pin = (intel_get_total_ram_mb() + 1) << 20;
> avail = (intel_get_avail_ram_mb() + 1) << 20;
> @@ -245,34 +243,40 @@ intel_get_total_pinnable_mem(void)
> */
> *can_mlock = (avail >> 1) + (avail >> 2);
> if (mlock(can_mlock, *can_mlock)) {
> - *can_mlock = 0;
> - goto out;
> + munmap(can_mlock, pin);
> + return MAP_FAILED;
> }
>
> for (uint64_t inc = 1024 << 20; inc >= 4 << 10; inc >>= 2) {
> - igt_debug("Testing mlock %'"PRIu64" B (%'"PRIu64"
> MiB)\n",
> - *can_mlock, *can_mlock >> 20);
> + uint64_t locked = *can_mlock;
> +
> + igt_debug("Testing mlock %'"PRIu64"B (%'"PRIu64"MiB) +
> %'"PRIu64"B\n",
> + locked, locked >> 20, inc);
>
> igt_fork(child, 1) {
> - for (uint64_t bytes = *can_mlock;
> - bytes <= pin;
> - bytes += inc) {
> - if (mlock(can_mlock, bytes))
> + uint64_t bytes = *can_mlock;
> +
> + while (bytes <= pin) {
> + if (mlock((void *)can_mlock + bytes,
> inc))
> break;
>
> - *can_mlock = bytes;
> + *can_mlock = bytes += inc;
> __sync_synchronize();
> }
> }
> __igt_waitchildren();
> - igt_assert(!mlock(can_mlock, *can_mlock));
> - }
>
> -out:
> - ret = *can_mlock;
> - munmap(can_mlock, pin);
> + if (*can_mlock > locked + inc) { /* Weird bit of mm/
> lore */
> + *can_mlock -= inc;
> + igt_debug("Claiming mlock %'"PRIu64"B
> (%'"PRIu64"MiB)\n",
> + *can_mlock, *can_mlock >> 20);
> + igt_assert(!mlock((void *)can_mlock + locked,
> + *can_mlock - locked));
> + }
> + }
>
> - return ret;
> + *total = pin;
> + return can_mlock;
> }
>
> static uint64_t vfs_file_max(void)
> diff --git a/tests/eviction_common.c b/tests/eviction_common.c
> index 321772ba7..a3b9e4167 100644
> --- a/tests/eviction_common.c
> +++ b/tests/eviction_common.c
> @@ -133,23 +133,24 @@ static void mlocked_evictions(int fd, struct
> igt_eviction_test_ops *ops,
> uint64_t surface_size,
> uint64_t surface_count)
> {
> + uint64_t sz, pin, total;
> void *mem;
> - uint64_t sz, pin_total;
>
> intel_require_memory(surface_count, surface_size, CHECK_RAM);
>
> - sz = surface_size*surface_count;
> - pin_total = intel_get_total_pinnable_mem();
> - igt_require(pin_total > sz);
> -
> - mem = mmap(NULL, pin_total, PROT_READ, MAP_SHARED | MAP_ANON,
> -1, 0);
> + mem = intel_get_total_pinnable_mem(&total);
> igt_assert(mem != MAP_FAILED);
> + pin = *(uint64_t *)mem;
> + igt_assert(!munlock(mem, pin));
> +
> + sz = surface_size * surface_count;
> + igt_require(pin > sz);
>
> igt_fork(child, 1) {
> uint32_t *bo;
> uint64_t n;
> int ret;
> - size_t lock = pin_total - sz;
> + size_t lock = pin - sz;
>
> bo = malloc(surface_count * sizeof(*bo));
> igt_assert(bo);
> @@ -186,7 +187,7 @@ static void mlocked_evictions(int fd, struct
> igt_eviction_test_ops *ops,
> }
> igt_waitchildren();
>
> - munmap(mem, pin_total);
> + munmap(mem, total);
> }
>
> static int swapping_evictions(int fd, struct igt_eviction_test_ops
> *ops,
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> index 84cb3b490..cd7cf9675 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -163,25 +163,14 @@ test_sysfs_reader(bool hibernate)
> static void
> test_shrink(int fd, unsigned int mode)
> {
> - void *mem;
> size_t size;
> + void *mem;
>
> gem_quiescent_gpu(fd);
> intel_purge_vm_caches(fd);
>
> - size = intel_get_total_pinnable_mem();
> - igt_require(size > 64 << 20);
> - size -= 64 << 20;
> -
> - mem = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_ANON, -1,
> 0);
> -
> - intel_purge_vm_caches(fd);
> -
> - igt_debug("Locking %'zu B (%'zu MiB)\n",
> - size, size >> 20);
> - igt_assert(!mlock(mem, size));
> - igt_info("Locked %'zu B (%'zu MiB)\n",
> - size, size >> 20);
> + mem = intel_get_total_pinnable_mem(&size);
> + igt_assert(mem != MAP_FAILED);
>
> intel_purge_vm_caches(fd);
> igt_system_suspend_autoresume(mode, SUSPEND_TEST_NONE);
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] lib: Incrementally mlock()
2019-02-27 16:29 [igt-dev] [PATCH i-g-t] lib: Incrementally mlock() Chris Wilson
` (2 preceding siblings ...)
2019-02-28 18:56 ` Caz Yokoyama
@ 2019-03-01 6:27 ` Ashutosh Dixit
3 siblings, 0 replies; 6+ messages in thread
From: Ashutosh Dixit @ 2019-03-01 6:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
On Wed, Feb 27, 2019 at 04:29:50PM +0000, Chris Wilson wrote:
>As we already have the previous portion of the mmap mlocked, we only
>need to mlock() the fresh portion for testing available memory.
>
>v2: Fixup the uint64_t pointer arithmetric and only use a single mmap to
>avoid subsequent mlock fail (for reasons unknown, but bet on mm/).
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>Cc: Caz Yokoyama <Caz.Yokoyama@intel.com>
Wondering how this patch was reviewed, tested and merged. The parent
process still dies with 100% certainty rendering the test useless:
# build/tests/i915_suspend --run-subtest shrink
IGT-Version: 1.23-g6be2dc8d (x86_64) (Linux: 5.0.0-rc5+ x86_64)
Starting subtest: shrink
child 0 died with signal 9, Killed
child 0 died with signal 9, Killed
child 0 died with signal 9, Killed
child 0 died with signal 9, Killed
child 0 died with signal 9, Killed
child 0 died with signal 9, Killed
Killed
>+ if (*can_mlock > locked + inc) { /* Weird bit of mm/ lore */
What is the meaning of this if check? The parent should mlock the
incremental amount the child has mlocked unconditionally as was done in the
previous version. If the objective was to kludge our way out of the parent
dying that objective doesn't seem to have been met either :(
-Ashutosh
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-01 6:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-27 16:29 [igt-dev] [PATCH i-g-t] lib: Incrementally mlock() Chris Wilson
2019-02-27 17:15 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2019-02-27 17:34 ` [igt-dev] [PATCH i-g-t] " Caz Yokoyama
2019-02-27 21:15 ` Chris Wilson
2019-02-28 18:56 ` Caz Yokoyama
2019-03-01 6:27 ` [igt-dev] [Intel-gfx] " Ashutosh Dixit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox