On 5/12/25 13:12, Hyejeong Choi wrote:
> smp_store_mb() inserts memory barrier after storing operation.
> It is different with what the comment is originally aiming so Null
> pointer dereference can be happened if memory update is reordered.
>
> Signed-off-by: Hyejeong Choi <hjeong.choi(a)samsung.com>
> ---
> drivers/dma-buf/dma-resv.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 5f8d010516f0..52af5c7430da 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -320,8 +320,9 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
> count++;
>
> dma_resv_list_set(fobj, i, fence, usage);
> - /* pointer update must be visible before we extend the num_fences */
> - smp_store_mb(fobj->num_fences, count);
> + /* fence update must be visible before we extend the num_fences */
> + smp_wmb();
> + WRITE_ONCE(fobj->num_fences, count);
The WRITE_ONCE isn't necessary since smp_wmb() implies a compiler barrier, but apart from that really good catch.
Can you modify the patch and re-send? I will be pushing it to -fixes ASAP.
Regards,
Christian.
> }
> EXPORT_SYMBOL(dma_resv_add_fence);
>
>
>
On 5/9/25 17:47, Matthew Brost wrote:
> On Fri, May 09, 2025 at 04:33:40PM +0100, Tvrtko Ursulin wrote:
>> Replace open-coded helper with the subsystem one.
>>
>
> You probably can just send this one by itself as it good cleanup and
> independent.
>
> Reviewed-by: Matthew Brost <matthew.brost(a)intel.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Any objections that I start to push those patches to drm-misc-next or do you want to take this one through the i915 branch?
Regards,
Christian.
>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_wait.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
>> index 7127e90c1a8f..991666fd9f85 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
>> @@ -106,11 +106,6 @@ static void fence_set_priority(struct dma_fence *fence,
>> rcu_read_unlock();
>> }
>>
>> -static inline bool __dma_fence_is_chain(const struct dma_fence *fence)
>> -{
>> - return fence->ops == &dma_fence_chain_ops;
>> -}
>> -
>> void i915_gem_fence_wait_priority(struct dma_fence *fence,
>> const struct i915_sched_attr *attr)
>> {
>> @@ -126,7 +121,7 @@ void i915_gem_fence_wait_priority(struct dma_fence *fence,
>>
>> for (i = 0; i < array->num_fences; i++)
>> fence_set_priority(array->fences[i], attr);
>> - } else if (__dma_fence_is_chain(fence)) {
>> + } else if (dma_fence_is_chain(fence)) {
>> struct dma_fence *iter;
>>
>> /* The chain is ordered; if we boost the last, we boost all */
>> --
>> 2.48.0
>>