[PATCH v3 3/5] drm/amdkfd: set activated flag true when event age unmatchs

[James Zhu]

On 2023-06-09 15:43, Felix Kuehling wrote:
>
> On 2023-06-08 13:07, James Zhu wrote:
>> Set waiter's activated flag true when event age unmatchs with 
>> last_event_age.
>>
>> Signed-off-by: James Zhu <James.Zhu at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_events.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> index c7689181cc22..4c6907507190 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> @@ -41,6 +41,7 @@ struct kfd_event_waiter {
>>       wait_queue_entry_t wait;
>>       struct kfd_event *event; /* Event to wait for */
>>       bool activated;         /* Becomes true when event is signaled */
>> +    bool event_age_enabled;  /* set to true when last_event_age is 
>> non-zero */
>>   };
>>     /*
>> @@ -797,9 +798,9 @@ static struct kfd_event_waiter 
>> *alloc_event_waiters(uint32_t num_events)
>>     static int init_event_waiter(struct kfd_process *p,
>>           struct kfd_event_waiter *waiter,
>> -        uint32_t event_id)
>> +        struct kfd_event_data *event_data)
>>   {
>> -    struct kfd_event *ev = lookup_event_by_id(p, event_id);
>> +    struct kfd_event *ev = lookup_event_by_id(p, event_data->event_id);
>>         if (!ev)
>>           return -EINVAL;
>> @@ -808,6 +809,13 @@ static int init_event_waiter(struct kfd_process *p,
>>       waiter->event = ev;
>>       waiter->activated = ev->signaled;
>>       ev->signaled = ev->signaled && !ev->auto_reset;
>> +
>> +    /* last_event_age = 0 reserved for backward compatible */
>> +    waiter->event_age_enabled = 
>> !!event_data->signal_event_data.last_event_age;
>> +    if (waiter->event_age_enabled &&
>> +        ev->event_age != event_data->signal_event_data.last_event_age)
>> +        WRITE_ONCE(waiter->activated, true);
>
> This needs to check the event type. Looking at 
> event_data->signal_event_data when this is not a signal event is 
> illegal, because it is aliased in a union with other event type data.
[JZ] Sure. I will add it. Thanks!
>
> Other than that, the series looks good to me now.
>
> Regards,
>   Felix
>
>
>> +
>>       if (!waiter->activated)
>>           add_wait_queue(&ev->wq, &waiter->wait);
>>       spin_unlock(&ev->lock);
>> @@ -948,8 +956,7 @@ int kfd_wait_on_events(struct kfd_process *p,
>>               goto out_unlock;
>>           }
>>   -        ret = init_event_waiter(p, &event_waiters[i],
>> -                    event_data.event_id);
>> +        ret = init_event_waiter(p, &event_waiters[i], &event_data);
>>           if (ret)
>>               goto out_unlock;
>>       }