* Re: [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock()
2019-02-22 2:03 [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock() Caz Yokoyama
@ 2019-02-21 20:25 ` Chris Wilson
2019-02-22 16:12 ` Ashutosh Dixit
2019-02-26 16:06 ` Caz Yokoyama
2 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2019-02-21 20:25 UTC (permalink / raw
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-02-22 02:03:57)
> As we already have the previous portion of the mmap mlocked, we only
> need to mlock() the fresh portion for testing available memory.
I liked my v2 much better that avoided the repeated mlock.
> Address calculation on mlock is fixed.
>
> When the parent process mlocks, i.e.,
> igt_assert(!mlock(can_mlock, *can_mlock));
> most likely killed.
>
> In suspend.c, OOM kills when 64MB less memory is mlocked.
>
> This patch does not make the test faster. It runs 300-900sec.
> I recommend followings.
>
> 1. Run patched i915_suspend@shrink only on full test(shard test ?)
> 2. add lighter test such as 1) mlock 3/4 of avail memory, 2) suspend-resume
That's completely missing the point. The point of the test is what
happens if we hit oom during suspend. If nothing happens during suspend,
it is just yet another suspend.
The complaint for the test was the sheer volume of dmesg spam along with
regularly locking up the system (and if we aren't locking up the system,
the test isn't doing its job...)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock()
@ 2019-02-22 2:03 Caz Yokoyama
2019-02-21 20:25 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Caz Yokoyama @ 2019-02-22 2:03 UTC (permalink / raw
To: 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.
Address calculation on mlock is fixed.
When the parent process mlocks, i.e.,
igt_assert(!mlock(can_mlock, *can_mlock));
most likely killed.
In suspend.c, OOM kills when 64MB less memory is mlocked.
This patch does not make the test faster. It runs 300-900sec.
I recommend followings.
1. Run patched i915_suspend@shrink only on full test(shard test ?)
2. add lighter test such as 1) mlock 3/4 of avail memory, 2) suspend-resume
2 constantly runs less than 5 sec. However, it does not suspend-resume
with exhausting all of system memory.
Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
---
lib/intel_os.c | 17 +++++++++--------
tests/i915/suspend.c | 7 +++++--
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/lib/intel_os.c b/lib/intel_os.c
index e1e31e23..44e852ee 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -250,22 +250,23 @@ intel_get_total_pinnable_mem(void)
}
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:
diff --git a/tests/i915/suspend.c b/tests/i915/suspend.c
index 84cb3b49..99ab68be 100644
--- a/tests/i915/suspend.c
+++ b/tests/i915/suspend.c
@@ -160,6 +160,9 @@ test_sysfs_reader(bool hibernate)
igt_stop_helper(&reader);
}
+#define MB (1 << 20)
+#define LESS_MEM (252*MB)
+
static void
test_shrink(int fd, unsigned int mode)
{
@@ -170,8 +173,8 @@ test_shrink(int fd, unsigned int mode)
intel_purge_vm_caches(fd);
size = intel_get_total_pinnable_mem();
- igt_require(size > 64 << 20);
- size -= 64 << 20;
+ igt_require(size > LESS_MEM);
+ size -= LESS_MEM;
mem = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_ANON, -1, 0);
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock()
2019-02-22 2:03 [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock() Caz Yokoyama
2019-02-21 20:25 ` Chris Wilson
@ 2019-02-22 16:12 ` Ashutosh Dixit
2019-02-22 17:28 ` Caz Yokoyama
2019-02-26 16:06 ` Caz Yokoyama
2 siblings, 1 reply; 5+ messages in thread
From: Ashutosh Dixit @ 2019-02-22 16:12 UTC (permalink / raw
To: Caz Yokoyama; +Cc: igt-dev@lists.freedesktop.org
On Thu, Feb 21 2019 at 07:03:57 PM, Caz Yokoyama <caz.yokoyama@intel.com> wrote:
> __igt_waitchildren();
> - igt_assert(!mlock(can_mlock, *can_mlock));
This is also wrong. The parent process needs to lock the memory for the
calculation of the amount of locked memory to be correct. Also explains
why this tests runs for so long with this patch.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock()
2019-02-22 16:12 ` Ashutosh Dixit
@ 2019-02-22 17:28 ` Caz Yokoyama
0 siblings, 0 replies; 5+ messages in thread
From: Caz Yokoyama @ 2019-02-22 17:28 UTC (permalink / raw
To: Ashutosh Dixit; +Cc: igt-dev@lists.freedesktop.org
On Fri, 2019-02-22 at 08:12 -0800, Ashutosh Dixit wrote:
> On Thu, Feb 21 2019 at 07:03:57 PM, Caz Yokoyama <
> caz.yokoyama@intel.com> wrote:
> > __igt_waitchildren();
> > - igt_assert(!mlock(can_mlock, *can_mlock));
>
> This is also wrong. The parent process needs to lock the memory for
> the
> calculation of the amount of locked memory to be correct. Also
I answered in my commit message as below.
When the parent process mlocks, i.e.,
igt_assert(!mlock(can_mlock, *can_mlock));
most likely killed.
"most likely" is not correct". "100% of probability" is.
With the bug, both parent and child mlock 8 times far beyond location.
mlock() may fail because it may beyond mmaped area.
With fixing the bug, the child updates (*can_mlock). Then the parent
mlocks(*can_mlock) and killed by OOM.
Only solution I found is 1) the parent does not mlock(*can_mlock),
2) mlock 252mb less memory.
BTW, dmesg has
[ 4605.251250]
[ 8318] 0 8318 497037 315802 2768896 194 1000
i915_suspend
[ 4605.251252]
[ 8320] 0 8320 497037 103550 1056768 194 1000
i915_suspend
[ 4605.251254] Out of memory: Kill process 8318 (i915_suspend) score
1317 or sacrifice child
[ 4605.251267] Killed process 8320 (i915_suspend) total-vm:1988148kB,
anon-rss:0kB, file-rss:0kB, shmem-rss:414200kB
[ 4605.251470] oom_reaper: reaped process 8320 (i915_suspend), now
anon-rss:0kB, file-rss:0kB, shmem-rss:414200kB
[ 4605.251560] Purging GPU memory, 0 pages freed, 2327 pages still
pinned.
[ 4605.251563] 1 and 0 pages still available in the bound and unbound
GPU page lists.
> explains
> why this tests runs for so long with this patch.
Correction. It runs 7-9 sec. I did not kill X server.
I'll do no further investigation for this test because this is
disabled.
-caz
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock()
2019-02-22 2:03 [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock() Caz Yokoyama
2019-02-21 20:25 ` Chris Wilson
2019-02-22 16:12 ` Ashutosh Dixit
@ 2019-02-26 16:06 ` Caz Yokoyama
2 siblings, 0 replies; 5+ messages in thread
From: Caz Yokoyama @ 2019-02-26 16:06 UTC (permalink / raw
To: igt-dev
As we discussed on the mailng list, please drop my patch.
-caz
On Fri, 2019-02-22 at 02:03 +0000, Caz Yokoyama wrote:
> As we already have the previous portion of the mmap mlocked, we only
> need to mlock() the fresh portion for testing available memory.
>
> Address calculation on mlock is fixed.
>
> When the parent process mlocks, i.e.,
> igt_assert(!mlock(can_mlock, *can_mlock));
> most likely killed.
>
> In suspend.c, OOM kills when 64MB less memory is mlocked.
>
> This patch does not make the test faster. It runs 300-900sec.
> I recommend followings.
>
> 1. Run patched i915_suspend@shrink only on full test(shard test ?)
> 2. add lighter test such as 1) mlock 3/4 of avail memory, 2) suspend-
> resume
>
> 2 constantly runs less than 5 sec. However, it does not suspend-
> resume
> with exhausting all of system memory.
>
> Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
> ---
> lib/intel_os.c | 17 +++++++++--------
> tests/i915/suspend.c | 7 +++++--
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index e1e31e23..44e852ee 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -250,22 +250,23 @@ intel_get_total_pinnable_mem(void)
> }
>
> 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:
> diff --git a/tests/i915/suspend.c b/tests/i915/suspend.c
> index 84cb3b49..99ab68be 100644
> --- a/tests/i915/suspend.c
> +++ b/tests/i915/suspend.c
> @@ -160,6 +160,9 @@ test_sysfs_reader(bool hibernate)
> igt_stop_helper(&reader);
> }
>
> +#define MB (1 << 20)
> +#define LESS_MEM (252*MB)
> +
> static void
> test_shrink(int fd, unsigned int mode)
> {
> @@ -170,8 +173,8 @@ test_shrink(int fd, unsigned int mode)
> intel_purge_vm_caches(fd);
>
> size = intel_get_total_pinnable_mem();
> - igt_require(size > 64 << 20);
> - size -= 64 << 20;
> + igt_require(size > LESS_MEM);
> + size -= LESS_MEM;
>
> mem = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_ANON, -1,
> 0);
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-26 15:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-22 2:03 [igt-dev] [igt PATCH v2 1/1] lib: Incrementally mlock() Caz Yokoyama
2019-02-21 20:25 ` Chris Wilson
2019-02-22 16:12 ` Ashutosh Dixit
2019-02-22 17:28 ` Caz Yokoyama
2019-02-26 16:06 ` Caz Yokoyama
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.