Re: [PATCH 6/6] sched_ext: Don't hold scx_tasks_lock for too long

[Tejun Heo]

Hello,

On Thu, Oct 10, 2024 at 02:12:37PM -0500, David Vernet wrote:
...
> >  static struct task_struct *scx_task_iter_next(struct scx_task_iter *iter)
> >  {
> >  	struct list_head *cursor = &iter->cursor.tasks_node;
> >  	struct sched_ext_entity *pos;
> >  
> > -	lockdep_assert_held(&scx_tasks_lock);
> > +	if (!(++iter->cnt % SCX_OPS_TASK_ITER_BATCH)) {
> > +		scx_task_iter_unlock(iter);
> > +		cpu_relax();
> 
> Could you explain why we need this cpu_relax()? I thought it was only
> necessary for busy-wait loops.

I don't think we need them with the current queued spinlock implementation
but back when the spinlocks were dumb, just unlocking and relocking could
still starve out others.

cc'ing Waiman who should know a lot better than me. Waiman, what's the
current state? When releasing a spinlock to give others a chance, can we
just do unlock/lock or is cpu_relax() still useful in some cases?

Thanks.

-- 
tejun