linux-kernel - Re: [PATCH 12/17] workqueue: Implement disable/enable for (delayed) work items

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJhGHyCOgrU9TC9o97LpvAEL2p+Dgt=uN0mo01Gr7HadVSD-Ag@mail.gmail.com>
Date: Tue, 20 Feb 2024 15:22:26 +0800
From: Lai Jiangshan <jiangshanlai@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org, 
	allen.lkml@...il.com, kernel-team@...a.com
Subject: Re: [PATCH 12/17] workqueue: Implement disable/enable for (delayed)
 work items

Hello, Tejun

On Sat, Feb 17, 2024 at 2:06 AM Tejun Heo <tj@...nel.org> wrote:

> - A work item carries 10bit disable count in work->data while not queued.
>   The access to the count is synchronized by the PENDING bit like all other
>   parts of work->data.

It is 16bit disable count in the code.


> @@ -2417,7 +2437,8 @@ bool queue_work_on(int cpu, struct workqueue_struct *wq,
>
>         local_irq_save(irq_flags);
>
> -       if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
> +       if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)) &&
> +           !clear_pending_if_disabled(work)) {
>                 __queue_work(cpu, wq, work);
>                 ret = true;
>         }
> @@ -2577,7 +2598,8 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
>         /* read the comment in __queue_work() */
>         local_irq_save(irq_flags);
>
> -       if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
> +       if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)) &&
> +           !clear_pending_if_disabled(work)) {
>                 __queue_delayed_work(cpu, wq, dwork, delay);
>                 ret = true;
>         }

It misses the same handling  at queue_work_node() and queue_rcu_work().

But it is bad idea to cancel or disable rcu work since it can be possibly
in the rcu's waiting list.

> @@ -4173,20 +4195,46 @@ bool flush_rcu_work(struct rcu_work *rwork)
>  }
>  EXPORT_SYMBOL(flush_rcu_work);
>

Thanks
Lai