[PATCH V2] drm/xe: Access port only after lock acquired

[Upadhyay, Tejas]

> -----Original Message-----
> From: Nirmoy Das <nirmoy.das at linux.intel.com>
> Sent: Wednesday, June 5, 2024 2:14 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>; intel-
> xe at lists.freedesktop.org
> Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Subject: Re: [PATCH V2] drm/xe: Access port only after lock acquired
> 
> Hi Tejas,
> 
> On 6/4/2024 12:45 PM, Tejas Upadhyay wrote:
> > Currently, port is accessed before lock of port is acquired.
> > As followed at other places, lets acquire lock before accessing for
> > even better sync up.
> 
> The lock is for protecting port->active list so think the code here is fine.
> 
> 
> Regards,
> 
> Nirmoy
> 
> > V2:
> >    - Add port->active check under lock
> >
> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_execlist.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_execlist.c
> b/drivers/gpu/drm/xe/xe_execlist.c
> > index db906117db6d..fcbc3bac1ebe 100644
> > --- a/drivers/gpu/drm/xe/xe_execlist.c
> > +++ b/drivers/gpu/drm/xe/xe_execlist.c
> > @@ -210,15 +210,16 @@ static void xe_execlist_port_wake_locked(struct
> xe_execlist_port *port,
> >
> >   static void xe_execlist_make_active(struct xe_execlist_exec_queue *exl)
> >   {
> > -	struct xe_execlist_port *port = exl->port;
> > +	struct xe_execlist_port *port;
> >   	enum xe_exec_queue_priority priority = exl->q-
> >sched_props.priority;
> >
> >   	XE_WARN_ON(priority == XE_EXEC_QUEUE_PRIORITY_UNSET);
> >   	XE_WARN_ON(priority < 0);
> > -	XE_WARN_ON(priority >= ARRAY_SIZE(exl->port->active));
> >
> > -	spin_lock_irq(&port->lock);
> > +	spin_lock_irq(&exl->port->lock);
> >
> > +	XE_WARN_ON(priority >= ARRAY_SIZE(exl->port->active));

My bad, I think here lock is protecting active_link list and port->active does not need any lock. Lets drop this change, its not required.

Thanks,
Tejas
> > +	port = exl->port;
> >   	if (exl->active_priority != priority &&
> >   	    exl->active_priority != XE_EXEC_QUEUE_PRIORITY_UNSET) {
> >   		/* Priority changed, move it to the right list */