Re: [PATCH] drm/xe: Use missing lock in relay_needs_worker

[Nirmoy Das <nirmoy.das@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

Hi Michal,

On 6/3/2024 4:19 PM, Michal Wajdeczko wrote:
>
> On 03.06.2024 10:17, Nirmoy Das wrote:
>> Add missing lock that is protecting relay->incoming_actions.
> good catch!
>
>> Cc: Michal Wajdeczko<michal.wajdeczko@intel.com>
>> Signed-off-by: Nirmoy Das<nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_relay.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_relay.c b/drivers/gpu/drm/xe/xe_guc_relay.c
>> index c3bbaf474f9a..ade6162dc259 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_relay.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_relay.c
>> @@ -761,7 +761,14 @@ static void relay_process_incoming_action(struct xe_guc_relay *relay)
>>   
>>   static bool relay_needs_worker(struct xe_guc_relay *relay)
>>   {
>> -	return !list_empty(&relay->incoming_actions);
>> +	bool is_empty;
>> +
>> +	spin_lock(&relay->lock);
>> +	is_empty = list_empty(&relay->incoming_actions);
>> +	spin_unlock(&relay->lock);
>> +
>> +	return !is_empty;
>> +
>>   }
> alternate solution could be to just rely on the new return value from
> relay_process_incoming_action() that could tell us if there was nothing
> to dequeue so then we wont start new worker (at potential cost of one
> extra worker cycle to see that), but this LGTM too, so
>
> Reviewed-by: Michal Wajdeczko<michal.wajdeczko@intel.com>

I did spent some time modifying relay_process_incoming_action() to 
return a bool but I am not sure which one is better.

I will for now pick this one :)


Thanks,

Nirmoy

[-- Attachment #2: Type: text/html, Size: 2677 bytes --]