Re: [PATCH v2] sched_ext: sync disable_irq_work in bpf_scx_unreg()

[Cheng-Yang Chou <yphbchou0911@gmail.com>]

Hi Richard,

On Fri, Apr 24, 2026 at 06:02:21PM +0800, Richard Cheng wrote:
[...]
> ---
>  kernel/sched/ext.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 012ca8bd70fb..ff42ac197bfd 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5921,6 +5921,20 @@ static void scx_disable(struct scx_sched *sch, enum scx_exit_kind kind)
>  		irq_work_queue(&sch->disable_irq_work);
>  }
>  
> +/**
> + * scx_flush_disable_work - flush the disable work and wait for it to finish
> + * @sch: the scheduler

Not to be picky here, but I think '@sch: the scheduler' is a bit vague.
Since nearly every 'sch' in ext.c is a scheduler instance,
maybe '@sch: sched to be flushed' would be more descriptive?

You can check more specific comments for 'sch' elsewhere in ext.c as well.
WDYT, thanks!

> + *
> + * sch->disable_work might still not queued, causing kthread_flush_work()
> + * as a noop. Syncing the irq_work first is required to guarantee the
> + * kthread work has been queued before waiting for it.
> + */
> +static void scx_flush_disable_work(struct scx_sched *sch)
> +{
> +	irq_work_sync(&sch->disable_irq_work);
> +	kthread_flush_work(&sch->disable_work);
> +}
> +
>  static void dump_newline(struct seq_buf *s)
>  {
>  	trace_sched_ext_dump("");
> @@ -6821,7 +6835,7 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  	 * completion. sch's base reference will be put by bpf_scx_unreg().
>  	 */
>  	scx_error(sch, "scx_root_enable() failed (%d)", ret);
> -	kthread_flush_work(&sch->disable_work);
> +	scx_flush_disable_work(sch);
>  	cmd->ret = 0;
>  }
>  
> @@ -7088,7 +7102,7 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
>  	percpu_up_write(&scx_fork_rwsem);
>  err_disable:
>  	mutex_unlock(&scx_enable_mutex);
> -	kthread_flush_work(&sch->disable_work);
> +	scx_flush_disable_work(sch);
>  	cmd->ret = 0;
>  }
>  
> @@ -7349,7 +7363,7 @@ static void bpf_scx_unreg(void *kdata, struct bpf_link *link)
>  	struct scx_sched *sch = rcu_dereference_protected(ops->priv, true);
>  
>  	scx_disable(sch, SCX_EXIT_UNREG);
> -	kthread_flush_work(&sch->disable_work);
> +	scx_flush_disable_work(sch);
>  	RCU_INIT_POINTER(ops->priv, NULL);
>  	kobject_put(&sch->kobj);
>  }
> -- 
> 2.43.0
> 

-- 
Cheers,
Cheng-Yang