Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp2174949rdb; Thu, 7 Dec 2023 23:37:55 -0800 (PST) X-Google-Smtp-Source: AGHT+IFtPQS4p6d+XhvOCezr6ijtq65ylOtEycX0QseQZTXwiGdEH8WuyIxjvl1b+uj9+Ti4Gjxw X-Received: by 2002:a92:c9cc:0:b0:35d:9c00:c348 with SMTP id k12-20020a92c9cc000000b0035d9c00c348mr2779844ilq.102.1702021074816; Thu, 07 Dec 2023 23:37:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702021074; cv=none; d=google.com; s=arc-20160816; b=jrHJdtQ3N+lZsk/foFXaXEZt7KKn3r8YFNOuVH6qhsIJw4qOaCyEq+ngkiULPRZUno GioTZOHYf38ye3FJsz+aVEN9Aimwr307VG1W+/kP/r+QN9GKdOpZYJS4Db1vVEEdntkU X7N+eBN0n+Zp4K8xHJ32S/rD4etdmwcTz5rLD7Aolrw38g2TRVuOgeolEgThA8E1zqfW je79OQY51GuPQ2HWJymnyxvR7TEmAXckaaS6eVPGz2kzZVgBnEhUIRZOtVF0lP5+tuHH x9jNjn4zzMrAgXbIIhUtXx7VGuIMWI/HC7RZqooNsAy99mjxcxHrNnvx0McwQfo7GzRf tS2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=/lQ92KtEiSjx5iNgTwfJK0PeI8bwxuHNEJX76kXNyWM=; fh=oEvLHNH8F/IW0KURaUJJLYt273/1asWYKcSvMOpB96I=; b=gtKv6IcJPbZyX1TGXMClZ+/XuQNXqr26J0qIKWNocRw+8EU8GxL388LPgwo/8glD7r tZm5qSH/VEhDqf3yAMWLyjEeDi69k6M/Uru6K+Kr7dByFSTo86bN7v6VzceBZcYRJKFe rRmcne9aOiogGA93A0xO1bOefcqAsoIINpUgXWRYs0+Pz1JrRj459dyV4Pnr8k7uQr2y UXO2zakoJ9QIC/s3jqnVwJG8CSQdNikk4R2JacVD/ckB9XpFx8N24fBWr+nBBpHU1/ue UiHCRdlNIKQ6oRV9ZnmJauC/cFoViNTkdxXxGxjubTl/0VnPwdaEZGzqi+A+e2j3hFtO kJ6g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b="P/pc92Wo"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id by4-20020a056a02058400b005c6be9a9841si1167603pgb.469.2023.12.07.23.37.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 23:37:54 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b="P/pc92Wo"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 24A4C8108341; Thu, 7 Dec 2023 23:37:52 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1573275AbjLHHhf (ORCPT + 99 others); Fri, 8 Dec 2023 02:37:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48812 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1573249AbjLHHhd (ORCPT ); Fri, 8 Dec 2023 02:37:33 -0500 Received: from mail-vs1-xe29.google.com (mail-vs1-xe29.google.com [IPv6:2607:f8b0:4864:20::e29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D442E172C for ; Thu, 7 Dec 2023 23:37:39 -0800 (PST) Received: by mail-vs1-xe29.google.com with SMTP id ada2fe7eead31-464dcdaa83bso418552137.0 for ; Thu, 07 Dec 2023 23:37:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702021059; x=1702625859; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/lQ92KtEiSjx5iNgTwfJK0PeI8bwxuHNEJX76kXNyWM=; b=P/pc92WoCCEVkMGOHnvHC5whU7ww/c2fFv4Vky9myRLOLdzwX2uqnAe8Ef/mDoXPrq 2qpc2W3wylwwImgMpENOq8IGzqvHUZ/P0ouTATxjqytKo0Plq1oqIIc1C9tUGWdLe93a btivXtEg84CDi+GnW8l6JppsVrMMkknJfqlcevRGgmCDQEmq33s7DHZ16DGswyEta5rk c5cTlzW8MOT2oReGvD+37Su6+nbHfCW39j88fJ1YXFs4Y0wD2CGg6WlxnFBQVF9xi7QJ ZGHcwxV2TUnBJ0Mi7t2euQBjQjQjcXWaCuZsxnq57zNRZM7IqESk9jd7DqiMlZlynF7M lCAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702021059; x=1702625859; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/lQ92KtEiSjx5iNgTwfJK0PeI8bwxuHNEJX76kXNyWM=; b=B3BpzkbuJS5cK6qxB2tZm13CCq2GquffHtoNoN/X4HCPAQPZF3lsh3rFbXqHPvc6aK t/C/pmvQQz3B1g9SzTqsKZLm9k2P43e00ZgmK+jMjm0eXZljzeWLPXJKfgPKxJLNFOro icBagGxtIWDeptxHXYZMwYoj0bjQcGc/45/zRrk4QN8mkt3tvh8zxOyAinJTVGbBE9fA kFJvcPuK8hYUC/M38GTFhC4GsXdTeRs4P61g6OC1OSbZlm7kY8RsVnolUQGIH9a0JBHw cBA1UNNDJKFTFeH2mJIaqmPnv/4htxD3413hFxLun9+3seDt8IPsuXAgkNKxRxoWViqd qBLg== X-Gm-Message-State: AOJu0Ywb8vdJwhVAiDVLQGAK8wjs/rVzGwkE9/q0e8zjWoPBxZnaaBOO rMKBWHTm8M6a1w0MwWTermkkA8GA9U2swukI/8kZpQ== X-Received: by 2002:a67:f7d4:0:b0:464:7f21:17d6 with SMTP id a20-20020a67f7d4000000b004647f2117d6mr2365331vsp.15.1702021058807; Thu, 07 Dec 2023 23:37:38 -0800 (PST) MIME-Version: 1.0 References: <20231206-rb-new-condvar-methods-v1-0-33a4cab7fdaa@google.com> <20231206-rb-new-condvar-methods-v1-2-33a4cab7fdaa@google.com> <1dd1a3e8-ef9a-4e89-891f-b49d82acc5f8@gmail.com> In-Reply-To: <1dd1a3e8-ef9a-4e89-891f-b49d82acc5f8@gmail.com> From: Alice Ryhl Date: Fri, 8 Dec 2023 08:37:27 +0100 Message-ID: Subject: Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout` To: Tiago Lam Cc: Miguel Ojeda , Alex Gaynor , Wedson Almeida Filho , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Peter Zijlstra , Ingo Molnar , Will Deacon , Waiman Long , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 07 Dec 2023 23:37:52 -0800 (PST) On Wed, Dec 6, 2023 at 6:05=E2=80=AFPM Tiago Lam wrote= : > > > On 06/12/2023 10:09, Alice Ryhl wrote: > [...] > > diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs > > index 9861c6749ad0..a6a6b6ab0c39 100644 > > --- a/rust/kernel/sync/condvar.rs > > +++ b/rust/kernel/sync/condvar.rs > > @@ -120,6 +120,63 @@ fn wait_internal(&self, wai= t_state: u32, guard: &mut Guar > > unsafe { bindings::finish_wait(self.wait_list.get(), wait.get= ()) }; > > } > > > > + /// Atomically releases the given lock (whose ownership is proven = by the guard) and puts the > > + /// thread to sleep. It wakes up when notified by [`CondVar::notif= y_one`] or > > + /// [`CondVar::notify_all`], or when the thread receives a signal. > > + /// > > + /// Returns whether there is a signal pending. > > + fn wait_internal_timeout( > > + &self, > > + wait_state: u32, > > + guard: &mut Guard<'_, T, B>, > > + timeout: u64, > > + ) -> u64 > > + where > > + T: ?Sized, > > + B: Backend, > > + { > > + let wait =3D Opaque::::uninit(); > > + > > + // SAFETY: `wait` points to valid memory. > > + unsafe { bindings::init_wait(wait.get()) }; > > + > > + // SAFETY: Both `wait` and `wait_list` point to valid memory. > > + unsafe { > > + bindings::prepare_to_wait_exclusive(self.wait_list.get(), = wait.get(), wait_state as _) > > + }; > > + > > + // SAFETY: Switches to another thread. > > + let timeout =3D > > + guard.do_unlocked(|| unsafe { bindings::schedule_timeout(t= imeout as _) as _ }); > > It looks like `schedule_timeout()` simply calls `schedule()` when the > timeout passed is `MAX_SCHEDULE_TIMEOUT`, so `wait_internal_timeout()` > could be merged together with the already existing `wait_internal()`, > where `wait_internal()` would always call `schedule_timeout()`? I may be > missing something, so just wondering why you decided to introduce > another method. Ah, nice! I didn't notice that I could combine them. > > + /// Releases the lock and waits for a notification in interruptibl= e mode. > > + /// > > + /// Atomically releases the given lock (whose ownership is proven = by the guard) and puts the > > + /// thread to sleep. It wakes up when notified by [`CondVar::notif= y_one`] or > > + /// [`CondVar::notify_all`], or when a timeout occurs, or when the= thread receives a signal. > > + /// > > + /// Returns whether there is a signal pending. > > + #[must_use =3D "wait_timeout returns if a signal is pending, so th= e caller must check the return value"] > > + pub fn wait_timeout( > > + &self, > > + guard: &mut Guard<'_, T, B>, > > + jiffies: u64, > > + ) -> CondVarTimeoutResult { > > Should this be called `wait_timeout_interruptable` instead, so that if > we need to add one using the `TASK_INTERRUPTIBLE` state later we don't > need to modfy it again? It also matches the > `schedule_timeout_interruptible` one in the kernel (although that's not > a reason to change it just in itself). I don't mind changing the names, but in this patch I was just consistent with what was already there. > > +/// The return type of `wait_timeout`. > > +pub enum CondVarTimeoutResult { > > + /// The timeout was reached. > > + Timeout, > > + /// Somebody woke us up. > > + Woken { > > + /// Remaining sleep duration. > > + jiffies: u64, > > + }, > > + /// A signal occurred. > > + Signal { > > + /// Remaining sleep duration. > > + jiffies: u64, > > + }, > > +} > > > Is `Signal` and `Woken` only going to hold a single value? Would it be > best represented as a tuple struct instead, like so? > > pub enum CondVarTimeoutResult { > /// The timeout was reached. > Timeout, > /// Somebody woke us up. > Woken (u64), > /// A signal occurred. > Signal (u64), > } I could do that, but I like the explicitly named version as it makes it clear that the unit is jiffies. Alice