* [PATCH] drm/i915: Fix the race between the GEM close and debugfs
@ 2020-09-14 11:00 ` Nikunj A. Dadhania
0 siblings, 0 replies; 7+ messages in thread
From: Nikunj A. Dadhania @ 2020-09-14 11:00 UTC (permalink / raw)
To: intel-gfx, dri-devel, chris, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Daniel Vetter
Cc: nikunj.dadhania
As we close GEM object and set file_priv to -EBADF which is protected
by ctx->mutex, populating the GEM debugfs info is not protected
and results in the crash shown below.
Make sure to protect the access to file_priv using ctx->mutex to avoid
race.
BUG: unable to handle page fault for address: ffffffffffffffff
RIP: 0010:i915_gem_object_info+0x26b/0x3eb
Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
Call Trace:
seq_read+0x162/0x3ca
full_proxy_read+0x5b/0x8d
__vfs_read+0x45/0x1b9
vfs_read+0xc9/0x15e
ksys_read+0x7e/0xde
do_syscall_64+0x54/0x7e
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7a1bdd34cf03
Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 784219962193..ea469168cd44 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
}
i915_gem_context_unlock_engines(ctx);
+ mutex_lock(&ctx->mutex);
if (!IS_ERR_OR_NULL(ctx->file_priv)) {
struct file_stats stats = {
.vm = rcu_access_pointer(ctx->vm),
@@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
print_file_stats(m, name, stats);
}
+ mutex_unlock(&ctx->mutex);
spin_lock(&i915->gem.contexts.lock);
list_safe_reset_next(ctx, cn, link);
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
@ 2020-09-14 11:00 ` Nikunj A. Dadhania
0 siblings, 0 replies; 7+ messages in thread
From: Nikunj A. Dadhania @ 2020-09-14 11:00 UTC (permalink / raw)
To: intel-gfx, dri-devel, chris, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Daniel Vetter
Cc: nikunj.dadhania
As we close GEM object and set file_priv to -EBADF which is protected
by ctx->mutex, populating the GEM debugfs info is not protected
and results in the crash shown below.
Make sure to protect the access to file_priv using ctx->mutex to avoid
race.
BUG: unable to handle page fault for address: ffffffffffffffff
RIP: 0010:i915_gem_object_info+0x26b/0x3eb
Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
Call Trace:
seq_read+0x162/0x3ca
full_proxy_read+0x5b/0x8d
__vfs_read+0x45/0x1b9
vfs_read+0xc9/0x15e
ksys_read+0x7e/0xde
do_syscall_64+0x54/0x7e
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7a1bdd34cf03
Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 784219962193..ea469168cd44 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
}
i915_gem_context_unlock_engines(ctx);
+ mutex_lock(&ctx->mutex);
if (!IS_ERR_OR_NULL(ctx->file_priv)) {
struct file_stats stats = {
.vm = rcu_access_pointer(ctx->vm),
@@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
print_file_stats(m, name, stats);
}
+ mutex_unlock(&ctx->mutex);
spin_lock(&i915->gem.contexts.lock);
list_safe_reset_next(ctx, cn, link);
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix the race between the GEM close and debugfs
2020-09-14 11:00 ` [Intel-gfx] " Nikunj A. Dadhania
(?)
@ 2020-09-14 16:00 ` Patchwork
-1 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-09-14 16:00 UTC (permalink / raw)
To: Nikunj A. Dadhania; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5828 bytes --]
== Series Details ==
Series: drm/i915: Fix the race between the GEM close and debugfs
URL : https://patchwork.freedesktop.org/series/81646/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_9006 -> Patchwork_18489
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/index.html
Known issues
------------
Here are the changes found in Patchwork_18489 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_flink_basic@flink-lifetime:
- fi-tgl-y: [PASS][1] -> [DMESG-WARN][2] ([i915#402]) +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
* igt@i915_selftest@live@execlists:
- fi-icl-y: [PASS][3] -> [INCOMPLETE][4] ([i915#2276])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-icl-y/igt@i915_selftest@live@execlists.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-icl-y/igt@i915_selftest@live@execlists.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-kefka: [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
- fi-icl-u2: [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-tgl-y: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-tgl-y/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-tgl-y/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
#### Possible fixes ####
* igt@gem_flink_basic@double-flink:
- fi-tgl-y: [DMESG-WARN][11] ([i915#402]) -> [PASS][12] +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-tgl-y/igt@gem_flink_basic@double-flink.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-tgl-y/igt@gem_flink_basic@double-flink.html
* igt@kms_busy@basic@flip:
- {fi-tgl-dsi}: [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-tgl-dsi/igt@kms_busy@basic@flip.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-tgl-dsi/igt@kms_busy@basic@flip.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-tgl-y: [DMESG-WARN][15] ([i915#1982]) -> [PASS][16] +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-tgl-y/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-tgl-y/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
#### Warnings ####
* igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
- fi-kbl-x1275: [DMESG-WARN][17] ([i915#62] / [i915#92]) -> [DMESG-WARN][18] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-kbl-x1275: [DMESG-WARN][19] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][20] ([i915#62] / [i915#92]) +3 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9006/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (45 -> 39)
------------------------------
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_9006 -> Patchwork_18489
CI-20190529: 20190529
CI_DRM_9006: 321a4bfb490be5dfa619bde041accb92676b797d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5781: 66766dd7cd99465d977ac07db8a2413dbbfe8d84 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18489: c5c693d63967c4bf246147cd33ed824583b5876a @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
c5c693d63967 drm/i915: Fix the race between the GEM close and debugfs
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18489/index.html
[-- Attachment #1.2: Type: text/html, Size: 7474 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
2020-09-14 11:00 ` [Intel-gfx] " Nikunj A. Dadhania
@ 2020-09-14 16:47 ` Tvrtko Ursulin
-1 siblings, 0 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2020-09-14 16:47 UTC (permalink / raw)
To: Nikunj A. Dadhania, intel-gfx, dri-devel, chris, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter
On 14/09/2020 12:00, Nikunj A. Dadhania wrote:
> As we close GEM object and set file_priv to -EBADF which is protected
> by ctx->mutex, populating the GEM debugfs info is not protected
> and results in the crash shown below.
>
> Make sure to protect the access to file_priv using ctx->mutex to avoid
> race.
>
> BUG: unable to handle page fault for address: ffffffffffffffff
> RIP: 0010:i915_gem_object_info+0x26b/0x3eb
> Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
> RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
> RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
> RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
> RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
> R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
> R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
> FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
> Call Trace:
> seq_read+0x162/0x3ca
> full_proxy_read+0x5b/0x8d
> __vfs_read+0x45/0x1b9
> vfs_read+0xc9/0x15e
> ksys_read+0x7e/0xde
> do_syscall_64+0x54/0x7e
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7a1bdd34cf03
>
> Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 784219962193..ea469168cd44 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
> }
> i915_gem_context_unlock_engines(ctx);
>
> + mutex_lock(&ctx->mutex);
> if (!IS_ERR_OR_NULL(ctx->file_priv)) {
> struct file_stats stats = {
> .vm = rcu_access_pointer(ctx->vm),
> @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
>
> print_file_stats(m, name, stats);
> }
> + mutex_unlock(&ctx->mutex);
>
> spin_lock(&i915->gem.contexts.lock);
> list_safe_reset_next(ctx, cn, link);
>
Fix is correct, but it looked familiar and indeed I found a fix for the
same issues back from July. Copied you on that one which now has an r-b.
This one can have it as well but please also copy stable.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
@ 2020-09-14 16:47 ` Tvrtko Ursulin
0 siblings, 0 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2020-09-14 16:47 UTC (permalink / raw)
To: Nikunj A. Dadhania, intel-gfx, dri-devel, chris, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter
On 14/09/2020 12:00, Nikunj A. Dadhania wrote:
> As we close GEM object and set file_priv to -EBADF which is protected
> by ctx->mutex, populating the GEM debugfs info is not protected
> and results in the crash shown below.
>
> Make sure to protect the access to file_priv using ctx->mutex to avoid
> race.
>
> BUG: unable to handle page fault for address: ffffffffffffffff
> RIP: 0010:i915_gem_object_info+0x26b/0x3eb
> Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
> RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
> RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
> RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
> RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
> R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
> R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
> FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
> Call Trace:
> seq_read+0x162/0x3ca
> full_proxy_read+0x5b/0x8d
> __vfs_read+0x45/0x1b9
> vfs_read+0xc9/0x15e
> ksys_read+0x7e/0xde
> do_syscall_64+0x54/0x7e
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7a1bdd34cf03
>
> Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 784219962193..ea469168cd44 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
> }
> i915_gem_context_unlock_engines(ctx);
>
> + mutex_lock(&ctx->mutex);
> if (!IS_ERR_OR_NULL(ctx->file_priv)) {
> struct file_stats stats = {
> .vm = rcu_access_pointer(ctx->vm),
> @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
>
> print_file_stats(m, name, stats);
> }
> + mutex_unlock(&ctx->mutex);
>
> spin_lock(&i915->gem.contexts.lock);
> list_safe_reset_next(ctx, cn, link);
>
Fix is correct, but it looked familiar and indeed I found a fix for the
same issues back from July. Copied you on that one which now has an r-b.
This one can have it as well but please also copy stable.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
2020-09-14 16:47 ` Tvrtko Ursulin
@ 2020-09-14 17:52 ` Nikunj A. Dadhania
-1 siblings, 0 replies; 7+ messages in thread
From: Nikunj A. Dadhania @ 2020-09-14 17:52 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx, dri-devel, chris, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter
On 9/14/2020 10:17 PM, Tvrtko Ursulin wrote:
>
> On 14/09/2020 12:00, Nikunj A. Dadhania wrote:
>> As we close GEM object and set file_priv to -EBADF which is protected
>> by ctx->mutex, populating the GEM debugfs info is not protected
>> and results in the crash shown below.
>>
>> Make sure to protect the access to file_priv using ctx->mutex to avoid
>> race.
>>
>> BUG: unable to handle page fault for address: ffffffffffffffff
>> RIP: 0010:i915_gem_object_info+0x26b/0x3eb
>> Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89
>> 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b
>> 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
>> RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
>> RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
>> RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
>> RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
>> R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
>> R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
>> FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
>> Call Trace:
>> seq_read+0x162/0x3ca
>> full_proxy_read+0x5b/0x8d
>> __vfs_read+0x45/0x1b9
>> vfs_read+0xc9/0x15e
>> ksys_read+0x7e/0xde
>> do_syscall_64+0x54/0x7e
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> RIP: 0033:0x7a1bdd34cf03
>>
>> Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 784219962193..ea469168cd44 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
>> }
>> i915_gem_context_unlock_engines(ctx);
>> + mutex_lock(&ctx->mutex);
>> if (!IS_ERR_OR_NULL(ctx->file_priv)) {
>> struct file_stats stats = {
>> .vm = rcu_access_pointer(ctx->vm),
>> @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
>> print_file_stats(m, name, stats);
>> }
>> + mutex_unlock(&ctx->mutex);
>> spin_lock(&i915->gem.contexts.lock);
>> list_safe_reset_next(ctx, cn, link);
>>
>
> Fix is correct, but it looked familiar and indeed I found a fix for the
> same issues back from July. Copied you on that one which now has an r-b.
Yes, saw your other email. Both are same, whichever gets applied is fine.
> This one can have it as well but please also copy stable.
Do I need to send the patch again with CC to stable?
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Thanks
Nikunj
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
@ 2020-09-14 17:52 ` Nikunj A. Dadhania
0 siblings, 0 replies; 7+ messages in thread
From: Nikunj A. Dadhania @ 2020-09-14 17:52 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx, dri-devel, chris, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter
On 9/14/2020 10:17 PM, Tvrtko Ursulin wrote:
>
> On 14/09/2020 12:00, Nikunj A. Dadhania wrote:
>> As we close GEM object and set file_priv to -EBADF which is protected
>> by ctx->mutex, populating the GEM debugfs info is not protected
>> and results in the crash shown below.
>>
>> Make sure to protect the access to file_priv using ctx->mutex to avoid
>> race.
>>
>> BUG: unable to handle page fault for address: ffffffffffffffff
>> RIP: 0010:i915_gem_object_info+0x26b/0x3eb
>> Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89
>> 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b
>> 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc
>> RSP: 0018:ffffac81c14cfc30 EFLAGS: 00010246
>> RAX: fffffffffffffff7 RBX: ffff95094429c218 RCX: ffff95096756c740
>> RDX: 0000000000000000 RSI: ffffffff919b93ee RDI: ffff95094429c218
>> RBP: ffffac81c14cfd58 R08: ffff9509746fab80 R09: 0000000000000000
>> R10: 0000000000000001 R11: 0000000000000000 R12: ffff9509753f8e80
>> R13: ffffac81c14cfc98 R14: ffff95094429c268 R15: ffffac81c14cfc88
>> FS: 00007a1bdcd52900(0000) GS:ffff950977e00000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: ffffffffffffffff CR3: 000000026b4e0000 CR4: 0000000000340ef0
>> Call Trace:
>> seq_read+0x162/0x3ca
>> full_proxy_read+0x5b/0x8d
>> __vfs_read+0x45/0x1b9
>> vfs_read+0xc9/0x15e
>> ksys_read+0x7e/0xde
>> do_syscall_64+0x54/0x7e
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> RIP: 0033:0x7a1bdd34cf03
>>
>> Signed-off-by: Nikunj A. Dadhania <nikunj.dadhania@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 784219962193..ea469168cd44 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m,
>> }
>> i915_gem_context_unlock_engines(ctx);
>> + mutex_lock(&ctx->mutex);
>> if (!IS_ERR_OR_NULL(ctx->file_priv)) {
>> struct file_stats stats = {
>> .vm = rcu_access_pointer(ctx->vm),
>> @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m,
>> print_file_stats(m, name, stats);
>> }
>> + mutex_unlock(&ctx->mutex);
>> spin_lock(&i915->gem.contexts.lock);
>> list_safe_reset_next(ctx, cn, link);
>>
>
> Fix is correct, but it looked familiar and indeed I found a fix for the
> same issues back from July. Copied you on that one which now has an r-b.
Yes, saw your other email. Both are same, whichever gets applied is fine.
> This one can have it as well but please also copy stable.
Do I need to send the patch again with CC to stable?
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Thanks
Nikunj
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-15 7:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 11:00 [PATCH] drm/i915: Fix the race between the GEM close and debugfs Nikunj A. Dadhania
2020-09-14 11:00 ` [Intel-gfx] " Nikunj A. Dadhania
2020-09-14 16:00 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2020-09-14 16:47 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
2020-09-14 16:47 ` Tvrtko Ursulin
2020-09-14 17:52 ` Nikunj A. Dadhania
2020-09-14 17:52 ` Nikunj A. Dadhania
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.