Hi Michal,
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