* [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path
@ 2023-03-17 0:22 Matthew Brost
2023-03-17 0:24 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Matthew Brost @ 2023-03-17 0:22 UTC (permalink / raw
To: intel-xe
We can't sleep in the CT fast but need to ensure we can access VRAM. Use
a trylock + reference counter check to ensure safe access to VRAM, if
either check fails, fall back to slow path.
VLK-45296
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.h | 9 ++++++++-
drivers/gpu/drm/xe/xe_guc_ct.c | 11 ++++++++++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
index 25c5087f5aad..0cc4f52098a1 100644
--- a/drivers/gpu/drm/xe/xe_device.h
+++ b/drivers/gpu/drm/xe/xe_device.h
@@ -95,12 +95,19 @@ static inline void xe_device_assert_mem_access(struct xe_device *xe)
XE_WARN_ON(!xe->mem_access.ref);
}
+static inline bool __xe_device_mem_access_ongoing(struct xe_device *xe)
+{
+ lockdep_assert_held(&xe->mem_access.lock);
+
+ return xe->mem_access.ref;
+}
+
static inline bool xe_device_mem_access_ongoing(struct xe_device *xe)
{
bool ret;
mutex_lock(&xe->mem_access.lock);
- ret = xe->mem_access.ref;
+ ret = __xe_device_mem_access_ongoing(xe);
mutex_unlock(&xe->mem_access.lock);
return ret;
diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index e5ed9022a0a2..bba0ef21c9e5 100644
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -1030,9 +1030,15 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
struct xe_device *xe = ct_to_xe(ct);
int len;
- if (!xe_device_in_fault_mode(xe) || !xe_device_mem_access_ongoing(xe))
+ if (!xe_device_in_fault_mode(xe))
return;
+ if (!mutex_trylock(&xe->mem_access.lock))
+ return;
+
+ if (!__xe_device_mem_access_ongoing(xe))
+ goto unlock;
+
spin_lock(&ct->fast_lock);
do {
len = g2h_read(ct, ct->fast_msg, true);
@@ -1040,6 +1046,9 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
g2h_fast_path(ct, ct->fast_msg, len);
} while (len > 0);
spin_unlock(&ct->fast_lock);
+
+unlock:
+ mutex_unlock(&xe->mem_access.lock);
}
/* Returns less than zero on error, 0 on done, 1 on more available */
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Intel-xe] ✓ CI.Patch_applied: success for drm/xe: Fix locking in CT fast path
2023-03-17 0:22 [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path Matthew Brost
@ 2023-03-17 0:24 ` Patchwork
2023-03-17 0:25 ` [Intel-xe] ✗ CI.KUnit: failure " Patchwork
2023-03-21 12:25 ` [Intel-xe] [PATCH] " Maarten Lankhorst
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2023-03-17 0:24 UTC (permalink / raw
To: Matthew Brost; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Fix locking in CT fast path
URL : https://patchwork.freedesktop.org/series/115301/
State : success
== Summary ==
=== Applying kernel patches on branch 'drm-xe-next' with base: ===
commit 68c0f7421b74cf51fe442bed6d4395d28ded5d7d
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>
AuthorDate: Tue Mar 14 15:56:44 2023 +0100
Commit: Thomas Hellström <thomas.hellstrom@linux.intel.com>
CommitDate: Thu Mar 16 15:16:00 2023 +0100
drm/xe/vm: Defer vm rebind until next exec if nothing to execute
If all compute engines of a vm in compute mode are idle,
defer a rebind to the next exec to avoid the VM unnecessarily trying
to make memory resident and compete with other VMs for available
memory space.
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
=== git am output follows ===
Applying: drm/xe: Fix locking in CT fast path
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Intel-xe] ✗ CI.KUnit: failure for drm/xe: Fix locking in CT fast path
2023-03-17 0:22 [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path Matthew Brost
2023-03-17 0:24 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
@ 2023-03-17 0:25 ` Patchwork
2023-03-21 12:25 ` [Intel-xe] [PATCH] " Maarten Lankhorst
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2023-03-17 0:25 UTC (permalink / raw
To: Matthew Brost; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Fix locking in CT fast path
URL : https://patchwork.freedesktop.org/series/115301/
State : failure
== Summary ==
+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:`.exit.text' referenced in section `.uml.exitcall.exit' of arch/um/drivers/virtio_uml.o: defined in discarded section `.exit.text' of arch/um/drivers/virtio_uml.o
/usr/bin/ld: drivers/gpu/drm/xe/display/intel_gmbus.o: in function `gmbus_func':
intel_gmbus.c:(.text+0xa): undefined reference to `i2c_bit_algo'
/usr/bin/ld: drivers/gpu/drm/xe/display/intel_gmbus.o: in function `gmbus_xfer':
intel_gmbus.c:(.text+0x14f7): undefined reference to `i2c_bit_algo'
collect2: error: ld returned 1 exit status
make[2]: *** [../scripts/Makefile.vmlinux:35: vmlinux] Error 1
make[1]: *** [/kernel/Makefile:1264: vmlinux] Error 2
make: *** [Makefile:242: __sub-make] Error 2
[00:24:40] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[00:24:45] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path
2023-03-17 0:22 [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path Matthew Brost
2023-03-17 0:24 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
2023-03-17 0:25 ` [Intel-xe] ✗ CI.KUnit: failure " Patchwork
@ 2023-03-21 12:25 ` Maarten Lankhorst
2023-03-21 14:34 ` Matthew Brost
2 siblings, 1 reply; 7+ messages in thread
From: Maarten Lankhorst @ 2023-03-21 12:25 UTC (permalink / raw
To: Matthew Brost, intel-xe
[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]
Hey,
I'm afraid this is not allowed, you can't take a mutex in an irq handler, not even a trylock.
From Documentation/locking/mutex-design.rst:
The mutex subsystem checks and enforces the following rules:
...
- Mutexes may not be used in hardware or software interrupt
contexts such as tasklets and timers.
Lockdep will likely still splat too as a result.
Cheers,
~Maarten
On 2023-03-17 01:22, Matthew Brost wrote:
> We can't sleep in the CT fast but need to ensure we can access VRAM. Use
> a trylock + reference counter check to ensure safe access to VRAM, if
> either check fails, fall back to slow path.
>
> VLK-45296
>
> Signed-off-by: Matthew Brost<matthew.brost@intel.com>
> ---
> drivers/gpu/drm/xe/xe_device.h | 9 ++++++++-
> drivers/gpu/drm/xe/xe_guc_ct.c | 11 ++++++++++-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 25c5087f5aad..0cc4f52098a1 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -95,12 +95,19 @@ static inline void xe_device_assert_mem_access(struct xe_device *xe)
> XE_WARN_ON(!xe->mem_access.ref);
> }
>
> +static inline bool __xe_device_mem_access_ongoing(struct xe_device *xe)
> +{
> + lockdep_assert_held(&xe->mem_access.lock);
> +
> + return xe->mem_access.ref;
> +}
> +
> static inline bool xe_device_mem_access_ongoing(struct xe_device *xe)
> {
> bool ret;
>
> mutex_lock(&xe->mem_access.lock);
> - ret = xe->mem_access.ref;
> + ret = __xe_device_mem_access_ongoing(xe);
> mutex_unlock(&xe->mem_access.lock);
>
> return ret;
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index e5ed9022a0a2..bba0ef21c9e5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1030,9 +1030,15 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> struct xe_device *xe = ct_to_xe(ct);
> int len;
>
> - if (!xe_device_in_fault_mode(xe) || !xe_device_mem_access_ongoing(xe))
> + if (!xe_device_in_fault_mode(xe))
> return;
>
> + if (!mutex_trylock(&xe->mem_access.lock))
> + return;
> +
> + if (!__xe_device_mem_access_ongoing(xe))
> + goto unlock;
> +
> spin_lock(&ct->fast_lock);
> do {
> len = g2h_read(ct, ct->fast_msg, true);
> @@ -1040,6 +1046,9 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> g2h_fast_path(ct, ct->fast_msg, len);
> } while (len > 0);
> spin_unlock(&ct->fast_lock);
> +
> +unlock:
> + mutex_unlock(&xe->mem_access.lock);
> }
>
> /* Returns less than zero on error, 0 on done, 1 on more available */
[-- Attachment #2: Type: text/html, Size: 2996 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path
2023-03-21 12:25 ` [Intel-xe] [PATCH] " Maarten Lankhorst
@ 2023-03-21 14:34 ` Matthew Brost
2023-03-21 21:14 ` Rodrigo Vivi
0 siblings, 1 reply; 7+ messages in thread
From: Matthew Brost @ 2023-03-21 14:34 UTC (permalink / raw
To: Maarten Lankhorst; +Cc: intel-xe
On Tue, Mar 21, 2023 at 01:25:51PM +0100, Maarten Lankhorst wrote:
> Hey,
>
> I'm afraid this is not allowed, you can't take a mutex in an irq handler, not even a trylock.
>
> From Documentation/locking/mutex-design.rst:
>
> The mutex subsystem checks and enforces the following rules:
> ...
> - Mutexes may not be used in hardware or software interrupt
> contexts such as tasklets and timers.
>
I wasn't aware of this byr DOC makes it clear this isn;t allowed.
> Lockdep will likely still splat too as a result.
>
Lockdep is happy which is very odd since clearly this isn't allowed per
the DOC.
Anyways, I'm thinking your atomic fix is needed but likely also need a
follow on to this patch as well something like:
xe_device_mem_access_get_if_active();
do CT fast path...
xe_device_mem_access_put_async();
The key being we can't sleep but also can't power down access to the
VRAM when the CT fast path is executing.
Matt
> Cheers,
> ~Maarten
>
> On 2023-03-17 01:22, Matthew Brost wrote:
> > We can't sleep in the CT fast but need to ensure we can access VRAM. Use
> > a trylock + reference counter check to ensure safe access to VRAM, if
> > either check fails, fall back to slow path.
> >
> > VLK-45296
> >
> > Signed-off-by: Matthew Brost<matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_device.h | 9 ++++++++-
> > drivers/gpu/drm/xe/xe_guc_ct.c | 11 ++++++++++-
> > 2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > index 25c5087f5aad..0cc4f52098a1 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -95,12 +95,19 @@ static inline void xe_device_assert_mem_access(struct xe_device *xe)
> > XE_WARN_ON(!xe->mem_access.ref);
> > }
> > +static inline bool __xe_device_mem_access_ongoing(struct xe_device *xe)
> > +{
> > + lockdep_assert_held(&xe->mem_access.lock);
> > +
> > + return xe->mem_access.ref;
> > +}
> > +
> > static inline bool xe_device_mem_access_ongoing(struct xe_device *xe)
> > {
> > bool ret;
> > mutex_lock(&xe->mem_access.lock);
> > - ret = xe->mem_access.ref;
> > + ret = __xe_device_mem_access_ongoing(xe);
> > mutex_unlock(&xe->mem_access.lock);
> > return ret;
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index e5ed9022a0a2..bba0ef21c9e5 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -1030,9 +1030,15 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> > struct xe_device *xe = ct_to_xe(ct);
> > int len;
> > - if (!xe_device_in_fault_mode(xe) || !xe_device_mem_access_ongoing(xe))
> > + if (!xe_device_in_fault_mode(xe))
> > return;
> > + if (!mutex_trylock(&xe->mem_access.lock))
> > + return;
> > +
> > + if (!__xe_device_mem_access_ongoing(xe))
> > + goto unlock;
> > +
> > spin_lock(&ct->fast_lock);
> > do {
> > len = g2h_read(ct, ct->fast_msg, true);
> > @@ -1040,6 +1046,9 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> > g2h_fast_path(ct, ct->fast_msg, len);
> > } while (len > 0);
> > spin_unlock(&ct->fast_lock);
> > +
> > +unlock:
> > + mutex_unlock(&xe->mem_access.lock);
> > }
> > /* Returns less than zero on error, 0 on done, 1 on more available */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path
2023-03-21 14:34 ` Matthew Brost
@ 2023-03-21 21:14 ` Rodrigo Vivi
2023-03-22 12:18 ` Maarten Lankhorst
0 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2023-03-21 21:14 UTC (permalink / raw
To: Matthew Brost; +Cc: intel-xe
On Tue, Mar 21, 2023 at 02:34:29PM +0000, Matthew Brost wrote:
> On Tue, Mar 21, 2023 at 01:25:51PM +0100, Maarten Lankhorst wrote:
> > Hey,
> >
> > I'm afraid this is not allowed, you can't take a mutex in an irq handler, not even a trylock.
> >
> > From Documentation/locking/mutex-design.rst:
> >
> > The mutex subsystem checks and enforces the following rules:
> > ...
> > - Mutexes may not be used in hardware or software interrupt
> > contexts such as tasklets and timers.
> >
>
> I wasn't aware of this byr DOC makes it clear this isn;t allowed.
>
> > Lockdep will likely still splat too as a result.
> >
>
> Lockdep is happy which is very odd since clearly this isn't allowed per
> the DOC.
This is strange... in general it is loud when you try mutex inside irq.
some .config missing? or maybe the trylock itself misleading lockdep?!
>
> Anyways, I'm thinking your atomic fix is needed
Yes, this is becoming un-avoidable. I would prefer some lock than atomic.
Maybe a spinlock?
Or we need to be really sure that there won't be any race where we end
with an access before the wakeup.
The runtime_pm doc even suggest that all the memory accesses should be
serialized instead of what we are trying to do currently with the
mem_access. Thoughts on if it is possible to serialize them on our cases?
check for the 'foo_' examples at Documentation/power/runtime_pm.txt
> but likely also need a
> follow on to this patch as well something like:
>
> xe_device_mem_access_get_if_active();
hmmm... I didn't want to grow the mem_access into an rpm wrapper for all
cases like we ended up in i915... but this might be unavoidable for this
case...
> do CT fast path...
> xe_device_mem_access_put_async();
>
> The key being we can't sleep but also can't power down access to the
> VRAM when the CT fast path is executing.
>
> Matt
>
> > Cheers,
> > ~Maarten
> >
> > On 2023-03-17 01:22, Matthew Brost wrote:
> > > We can't sleep in the CT fast but need to ensure we can access VRAM. Use
> > > a trylock + reference counter check to ensure safe access to VRAM, if
> > > either check fails, fall back to slow path.
> > >
> > > VLK-45296
> > >
> > > Signed-off-by: Matthew Brost<matthew.brost@intel.com>
> > > ---
> > > drivers/gpu/drm/xe/xe_device.h | 9 ++++++++-
> > > drivers/gpu/drm/xe/xe_guc_ct.c | 11 ++++++++++-
> > > 2 files changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > > index 25c5087f5aad..0cc4f52098a1 100644
> > > --- a/drivers/gpu/drm/xe/xe_device.h
> > > +++ b/drivers/gpu/drm/xe/xe_device.h
> > > @@ -95,12 +95,19 @@ static inline void xe_device_assert_mem_access(struct xe_device *xe)
> > > XE_WARN_ON(!xe->mem_access.ref);
> > > }
> > > +static inline bool __xe_device_mem_access_ongoing(struct xe_device *xe)
> > > +{
> > > + lockdep_assert_held(&xe->mem_access.lock);
> > > +
> > > + return xe->mem_access.ref;
> > > +}
> > > +
> > > static inline bool xe_device_mem_access_ongoing(struct xe_device *xe)
> > > {
> > > bool ret;
> > > mutex_lock(&xe->mem_access.lock);
> > > - ret = xe->mem_access.ref;
> > > + ret = __xe_device_mem_access_ongoing(xe);
> > > mutex_unlock(&xe->mem_access.lock);
> > > return ret;
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > index e5ed9022a0a2..bba0ef21c9e5 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > @@ -1030,9 +1030,15 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> > > struct xe_device *xe = ct_to_xe(ct);
> > > int len;
> > > - if (!xe_device_in_fault_mode(xe) || !xe_device_mem_access_ongoing(xe))
> > > + if (!xe_device_in_fault_mode(xe))
> > > return;
> > > + if (!mutex_trylock(&xe->mem_access.lock))
> > > + return;
> > > +
> > > + if (!__xe_device_mem_access_ongoing(xe))
> > > + goto unlock;
> > > +
> > > spin_lock(&ct->fast_lock);
> > > do {
> > > len = g2h_read(ct, ct->fast_msg, true);
> > > @@ -1040,6 +1046,9 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
> > > g2h_fast_path(ct, ct->fast_msg, len);
> > > } while (len > 0);
> > > spin_unlock(&ct->fast_lock);
> > > +
> > > +unlock:
> > > + mutex_unlock(&xe->mem_access.lock);
> > > }
> > > /* Returns less than zero on error, 0 on done, 1 on more available */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path
2023-03-21 21:14 ` Rodrigo Vivi
@ 2023-03-22 12:18 ` Maarten Lankhorst
0 siblings, 0 replies; 7+ messages in thread
From: Maarten Lankhorst @ 2023-03-22 12:18 UTC (permalink / raw
To: Rodrigo Vivi, Matthew Brost; +Cc: intel-xe
Hey,
On 2023-03-21 22:14, Rodrigo Vivi wrote:
> On Tue, Mar 21, 2023 at 02:34:29PM +0000, Matthew Brost wrote:
>> On Tue, Mar 21, 2023 at 01:25:51PM +0100, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> I'm afraid this is not allowed, you can't take a mutex in an irq handler, not even a trylock.
>>>
>>> From Documentation/locking/mutex-design.rst:
>>>
>>> The mutex subsystem checks and enforces the following rules:
>>> ...
>>> - Mutexes may not be used in hardware or software interrupt
>>> contexts such as tasklets and timers.
>>>
>> I wasn't aware of this byr DOC makes it clear this isn;t allowed.
>>
>>> Lockdep will likely still splat too as a result.
>>>
>> Lockdep is happy which is very odd since clearly this isn't allowed per
>> the DOC.
> This is strange... in general it is loud when you try mutex inside irq.
> some .config missing? or maybe the trylock itself misleading lockdep?!
It's not allowed because of PREEMPT_RT, which is upstream. The mutex is
swapped with a PI-mutex, and if the irq is triggered from idle, anyone
may boost the idle task.
>> Anyways, I'm thinking your atomic fix is needed
> Yes, this is becoming un-avoidable. I would prefer some lock than atomic.
> Maybe a spinlock?
> Or we need to be really sure that there won't be any race where we end
> with an access before the wakeup.
>
> The runtime_pm doc even suggest that all the memory accesses should be
> serialized instead of what we are trying to do currently with the
> mem_access. Thoughts on if it is possible to serialize them on our cases?
>
> check for the 'foo_' examples at Documentation/power/runtime_pm.txt
I think the current xe_device_mem() is possibly too generic. We should
check for what usecases the workaround is required, and we could use
several mechanisms to prevent the need for 1 all-encompassing workaround
implementation.
For example runtime pm could be used for jobs and job completion, which
makes most sense there, same probably for active mmapings.
By looking at them separately, it might be possible to come up with
something more suitable and granular.
~Maarten
>> but likely also need a
>> follow on to this patch as well something like:
>>
>> xe_device_mem_access_get_if_active();
> hmmm... I didn't want to grow the mem_access into an rpm wrapper for all
> cases like we ended up in i915... but this might be unavoidable for this
> case...
>
>> do CT fast path...
>> xe_device_mem_access_put_async();
>>
>> The key being we can't sleep but also can't power down access to the
>> VRAM when the CT fast path is executing.
>>
>> Matt
>>
>>> Cheers,
>>> ~Maarten
>>>
>>> On 2023-03-17 01:22, Matthew Brost wrote:
>>>> We can't sleep in the CT fast but need to ensure we can access VRAM. Use
>>>> a trylock + reference counter check to ensure safe access to VRAM, if
>>>> either check fails, fall back to slow path.
>>>>
>>>> VLK-45296
>>>>
>>>> Signed-off-by: Matthew Brost<matthew.brost@intel.com>
>>>> ---
>>>> drivers/gpu/drm/xe/xe_device.h | 9 ++++++++-
>>>> drivers/gpu/drm/xe/xe_guc_ct.c | 11 ++++++++++-
>>>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
>>>> index 25c5087f5aad..0cc4f52098a1 100644
>>>> --- a/drivers/gpu/drm/xe/xe_device.h
>>>> +++ b/drivers/gpu/drm/xe/xe_device.h
>>>> @@ -95,12 +95,19 @@ static inline void xe_device_assert_mem_access(struct xe_device *xe)
>>>> XE_WARN_ON(!xe->mem_access.ref);
>>>> }
>>>> +static inline bool __xe_device_mem_access_ongoing(struct xe_device *xe)
>>>> +{
>>>> + lockdep_assert_held(&xe->mem_access.lock);
>>>> +
>>>> + return xe->mem_access.ref;
>>>> +}
>>>> +
>>>> static inline bool xe_device_mem_access_ongoing(struct xe_device *xe)
>>>> {
>>>> bool ret;
>>>> mutex_lock(&xe->mem_access.lock);
>>>> - ret = xe->mem_access.ref;
>>>> + ret = __xe_device_mem_access_ongoing(xe);
>>>> mutex_unlock(&xe->mem_access.lock);
>>>> return ret;
>>>> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>>>> index e5ed9022a0a2..bba0ef21c9e5 100644
>>>> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
>>>> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>>>> @@ -1030,9 +1030,15 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
>>>> struct xe_device *xe = ct_to_xe(ct);
>>>> int len;
>>>> - if (!xe_device_in_fault_mode(xe) || !xe_device_mem_access_ongoing(xe))
>>>> + if (!xe_device_in_fault_mode(xe))
>>>> return;
>>>> + if (!mutex_trylock(&xe->mem_access.lock))
>>>> + return;
>>>> +
>>>> + if (!__xe_device_mem_access_ongoing(xe))
>>>> + goto unlock;
>>>> +
>>>> spin_lock(&ct->fast_lock);
>>>> do {
>>>> len = g2h_read(ct, ct->fast_msg, true);
>>>> @@ -1040,6 +1046,9 @@ void xe_guc_ct_fast_path(struct xe_guc_ct *ct)
>>>> g2h_fast_path(ct, ct->fast_msg, len);
>>>> } while (len > 0);
>>>> spin_unlock(&ct->fast_lock);
>>>> +
>>>> +unlock:
>>>> + mutex_unlock(&xe->mem_access.lock);
>>>> }
>>>> /* Returns less than zero on error, 0 on done, 1 on more available */
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-22 12:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-17 0:22 [Intel-xe] [PATCH] drm/xe: Fix locking in CT fast path Matthew Brost
2023-03-17 0:24 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
2023-03-17 0:25 ` [Intel-xe] ✗ CI.KUnit: failure " Patchwork
2023-03-21 12:25 ` [Intel-xe] [PATCH] " Maarten Lankhorst
2023-03-21 14:34 ` Matthew Brost
2023-03-21 21:14 ` Rodrigo Vivi
2023-03-22 12:18 ` Maarten Lankhorst
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.