Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Jun 2023 18:30:02 +0100
From:   Gary Guo <gary@garyguo.net>
To:     Alice Ryhl <aliceryhl@google.com>
Cc:     rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Tejun Heo <tj@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        "=?UTF-8?B?QmrDtnJu?= Roy Baron" <bjorn3_gh@protonmail.com>,
        Benno Lossin <benno.lossin@proton.me>,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        Wedson Almeida Filho <walmeida@microsoft.com>,
        Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Subject: Re: [PATCH v2 4/8] rust: workqueue: define built-in queues
Message-ID: <20230601183002.237a31fa.gary@garyguo.net>
In-Reply-To: <20230601134946.3887870-5-aliceryhl@google.com>
References: <20230601134946.3887870-1-aliceryhl@google.com>
        <20230601134946.3887870-5-aliceryhl@google.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?i0qSQWwDvPjBEPTm1/qYCNmIYSegYC6GARnfZZUUVjhf/Z82mZ2XwjlMPDBf?=
 =?us-ascii?Q?j2xzzG7r2Ke8rT4qaIAWvpdauGPsLnint5nQrunRRqiqF5wucvtuJ+kiD3wo?=
 =?us-ascii?Q?jm+JS8vuUVBRI8P44unHEsthDTZcQpsrRVFuWLLh0SYNfaqWpPa+U4+tspZN?=
 =?us-ascii?Q?yNs3cM0kL+foTPgOI+TKkPMRExIjkq5qivtcap1fVJUikGqAna+Q0Juu1+nR?=
 =?us-ascii?Q?9ucAppHWUdOXNe8UW7LowUpz88fl6p0wweK82whyKpDzrhlp90cH4OYrUaoq?=
 =?us-ascii?Q?k7qbT9FyfyGpzlyy5rDh8ZpJvWN3H7RnVUqThlkX16e9H4i5OHVC2V00g1M6?=
 =?us-ascii?Q?+wzHbhegM3/ER+XuZIJzdbFzlApwGw66DrPTOeWlVzC/25QGLm0YiccPFJuY?=
 =?us-ascii?Q?bTHP/2Puzd11ZKQLzOlfI68NO3wlelc0LdG0A6geZAiBKm7sHyb8lEpfonCq?=
 =?us-ascii?Q?3AlAnIvo45JTmF5eew+W/UZmKiTJuNFUcaIuN1SdfsUWudfTR10PtshaNUHm?=
 =?us-ascii?Q?nrvpOhwS8KRXZyUPtAkTCFG+NFbyr68QFsw7/gazyeO1Z0ZGyHcarDZUGnUC?=
 =?us-ascii?Q?s0MBw/ZuklSWQk37f1sJog78BQkF9kbtuo7Hg6fy673H5wnzm1hAx2+HDhDm?=
 =?us-ascii?Q?OPN6v8ioU3JGlylOyBykSbSXfKPHBVON2pNZHNru8DTfRapHQorOodVE/sIs?=
 =?us-ascii?Q?IxFDbht6mlgnOYmcnuZN3jXGu4v5+0oUOiNVKAd+D7dctJX1mKxld+S1zSmO?=
 =?us-ascii?Q?9xVGdGlysrYSeKUJGGeHsWfKuZQuhEQ2JWJ+I/FxeFOHGyRbsRI8EFJhaZ3L?=
 =?us-ascii?Q?a4qBfURoe+51VaVqy6Uxn+l6AhCBBkfc55oiNoPUV8LFYy97kyzT/FtGEBnj?=
 =?us-ascii?Q?w1mknLI5ZRdpALR1nmkviZOglNVLmlOA2ZbUCo/gNwU7oMJdC5dfxoIFp0SH?=
 =?us-ascii?Q?i+GN3355Go3oInJCwk/PPjF3GqN/sHmhWrbNs0N4We0KcuGkKVXHNCDCAJY2?=
 =?us-ascii?Q?ohCOcVkdgD18GwIQPcEciPaI7BnuXBIuJ5BkbcfYhm3C8kmOf5FRk27RnYSf?=
 =?us-ascii?Q?qawzyJlAtwaeKNFrpunCfRz43TYveH66dZCtrFu/jsTCTvZxQFhInq2lN7Dm?=
 =?us-ascii?Q?Vvsxa1XczOgp2ZLM99hj/MvPppF9WVFLvrJHGfICBpvSMlrYt9ibq1JvtGOr?=
 =?us-ascii?Q?nmGUz9G/5JmMwN605XW4onhCfQprZ9OSAhZeN9Q0drEPxgqcp/2HMkoFXhel?=
 =?us-ascii?Q?BwBxo+aTFiRvMKIk0LjEhCIDHIaDMdxyE0r5DxaL67DVR4W1wR1R91ifTAbv?=
 =?us-ascii?Q?FOc17pasOfDst5vn1GWMBKjEuevu1qpmXo5LiKVM4+J9rGAWbvSKuCf+qnnp?=
 =?us-ascii?Q?EwaFz3iIdyMRDT7NBbROWPqYvQmLn1O6M346dCxmW2qdXkzZcadu0ic3QcFQ?=
 =?us-ascii?Q?nzj4W+SEaWIapzD6q3j1Ot65Ss5XDX05rWB1XAgpNVcTnlM7sd9NqKG9+5Nf?=
 =?us-ascii?Q?vjUZVp01+bjQgLgPG15cLTocrVZTvL61GSqrYpZvqBLXKiFZAplMoqLsM/ig?=
 =?us-ascii?Q?A47OWAszJoQUNva0TnMhMDSxoGZjyZz47BrRSnxH?=
X-OriginatorOrg: garyguo.net
X-MS-Exchange-CrossTenant-Network-Message-Id: bb36c1b3-440f-49c2-7472-08db62c5d640
X-MS-Exchange-CrossTenant-AuthSource: LO2P265MB5183.GBRP265.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Jun 2023 17:30:04.7191
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bbc898ad-b10f-4e10-8552-d9377b823d45
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Ik5EdyK1HO04EnVWrdYI65WZjwc/xJdmIq3pmt7kXWwUp7qDwJl8Grvm7NgThbO0VsZNabzoZJ40c9CtRHi1fQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LO6P265MB6987
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu,  1 Jun 2023 13:49:42 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> We provide these methods because it lets us access these queues from
> Rust without using unsafe code.
> 
> These methods return `&'static Queue`. References annotated with the
> 'static lifetime are used when the referent will stay alive forever.
> That is ok for these queues because they are global variables and cannot
> be destroyed.
> 
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Co-developed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

This looks fine to me, so:

Reviewed-by: Gary Guo <gary@garyguo.net>

Just one question about style: would people prefer:

	kernel::workqueue::system().enqueue(...)

or

	use kernel::workqueue::Queue;
	Queue::system().enqueue(...)

?

> ---
>  rust/kernel/workqueue.rs | 65 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 9c630840039b..e37820f253f6 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -105,3 +105,68 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
>      where
>          F: FnOnce(*mut bindings::work_struct) -> bool;
>  }
> +
> +/// Returns the system work queue (`system_wq`).
> +///
> +/// It is the one used by `schedule[_delayed]_work[_on]()`. Multi-CPU multi-threaded. There are
> +/// users which expect relatively short queue flush time.
> +///
> +/// Callers shouldn't queue work items which can run for too long.
> +pub fn system() -> &'static Queue {
> +    // SAFETY: `system_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_wq) }
> +}
> +
> +/// Returns the system high-priority work queue (`system_highpri_wq`).
> +///
> +/// It is similar to the one returned by [`system`] but for work items which require higher
> +/// scheduling priority.
> +pub fn system_highpri() -> &'static Queue {
> +    // SAFETY: `system_highpri_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_highpri_wq) }
> +}
> +
> +/// Returns the system work queue for potentially long-running work items (`system_long_wq`).
> +///
> +/// It is similar to the one returned by [`system`] but may host long running work items. Queue
> +/// flushing might take relatively long.
> +pub fn system_long() -> &'static Queue {
> +    // SAFETY: `system_long_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_long_wq) }
> +}
> +
> +/// Returns the system unbound work queue (`system_unbound_wq`).
> +///
> +/// Workers are not bound to any specific CPU, not concurrency managed, and all queued work items
> +/// are executed immediately as long as `max_active` limit is not reached and resources are
> +/// available.
> +pub fn system_unbound() -> &'static Queue {
> +    // SAFETY: `system_unbound_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_unbound_wq) }
> +}
> +
> +/// Returns the system freezable work queue (`system_freezable_wq`).
> +///
> +/// It is equivalent to the one returned by [`system`] except that it's freezable.
> +pub fn system_freezable() -> &'static Queue {
> +    // SAFETY: `system_freezable_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_freezable_wq) }
> +}
> +
> +/// Returns the system power-efficient work queue (`system_power_efficient_wq`).
> +///
> +/// It is inclined towards saving power and is converted to "unbound" variants if the
> +/// `workqueue.power_efficient` kernel parameter is specified; otherwise, it is similar to the one
> +/// returned by [`system`].
> +pub fn system_power_efficient() -> &'static Queue {
> +    // SAFETY: `system_power_efficient_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_power_efficient_wq) }
> +}
> +
> +/// Returns the system freezable power-efficient work queue (`system_freezable_power_efficient_wq`).
> +///
> +/// It is similar to the one returned by [`system_power_efficient`] except that is freezable.
> +pub fn system_freezable_power_efficient() -> &'static Queue {
> +    // SAFETY: `system_freezable_power_efficient_wq` is a C global, always available.
> +    unsafe { Queue::from_raw(bindings::system_freezable_power_efficient_wq) }
> +}