Re: [PATCH] workqueue: Process rescuer work items one-by-one using a positional marker

[Tejun Heo <tj@kernel.org>]

Hello, Lai.

On Fri, Nov 14, 2025 at 12:34:26AM +0800, Lai Jiangshan wrote:
> +static void insert_mayday_pos(struct pool_workqueue *pwq, struct work_struct *next)
> +{
> +	unsigned int work_flags;
> +	unsigned int work_color;
> +
> +	__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&pwq->mayday_pos_work));

Maybe use test_and_set_bit() here like normal work item?

> +static void remove_mayday_pos(struct pool_workqueue *pwq)
> +{
> +	list_del_init(&pwq->mayday_pos_work.entry);
> +	pwq_dec_nr_in_flight(pwq, *work_data_bits(&pwq->mayday_pos_work));
> +	INIT_WORK(&pwq->mayday_pos_work, mayday_pos_func);

and maybe we can init the work item once and

>  static void send_mayday(struct work_struct *work)
>  {
>  	struct pool_workqueue *pwq = get_work_pwq(work);
> @@ -2992,6 +3037,9 @@ static void send_mayday(struct work_struct *work)
>  	if (!wq->rescuer)
>  		return;
>  
> +	if (!work_pending(&pwq->mayday_pos_work))

drop this conditional too?

Also, I wonder whether it'd be simpler to think about if we just exclude the
work item from flush color management. e.g. we can just flag the work item
and then make work scanning skip them, so that they really are just markers;
then, we don't have to worry about colors or othre states at all. We just
insert the marker and use it purely as iteration marker.

> +				/* reset the position and handle the assigned work */
> +				if (list_next_entry(&pwq->mayday_pos_work, entry) != n) {
> +					remove_mayday_pos(pwq);
> +					insert_mayday_pos(pwq, n);

and this would become simple list_move_tail().

Thanks.

-- 
tejun