From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f73.google.com (mail-wm1-f73.google.com [209.85.128.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5421534028D for ; Wed, 4 Mar 2026 21:37:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.73 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772660269; cv=none; b=sCzKJF6E2uOWBH98NVksOAv2axhJvMSKlv/rSFbfUXhIyKZPB3UXqZp/rEyS1pX/h+/7N2L7rlcmqC5BuCqFIU1BwZieRBY2VNbUda6TbJGTXJdpedwGrndL0v+AIHpjcRCae4ImbMieQDZR4w7UOi6KaHtUNKxdkUg5ahMnx3Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772660269; c=relaxed/simple; bh=DHFzucvKQo9mJnpiOC/mpjemhEuJZ6EV4obLrcNxNYQ=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=F8N0Lcv9OlCzOH/fAn42pF2TfuGmZQMkem1PApNhOLlRKNTSK9JDQAc0y/KSMvvdJMBA2fNcdCc7y9ybvmpSK52qUtnjeO/KZOkto5QgUHSJfgoMcSDXY1t8twWRKBPwzio3noZkiUpt3qP2Pql2zGVZobDyTZMgHDE6Gkm2GoY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=PeOv4i62; arc=none smtp.client-ip=209.85.128.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="PeOv4i62" Received: by mail-wm1-f73.google.com with SMTP id 5b1f17b1804b1-4836b7c302fso75279585e9.1 for ; Wed, 04 Mar 2026 13:37:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1772660267; x=1773265067; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=H3B6twlxs7RG5L5HQ0RR3EQNIcTaBDOh7BWXSLk64tU=; b=PeOv4i62mPdMN5GCFB4SI/nked8GOPruvrh9hLhH7seF6Tu+9ZCDjzdlYqv62xeyEW w4bltHeaJRElv1u9HArZLc8VCUn7rYhZEHNBN0vtn61qpT32va6U703RD/F9lQd97qPv cp/DAmuziaoS1QaGnaxh0eRi8xYxxqPEWV0XP0WrojV2DFLpdt03C1DRawQoDmqTevaQ GNHzje+jMWE/zSGPzDR6ZeCz1+bQAzSzQ/hAH+dZTOG/otHkGxknlSPeVxfv7JHpuP7c E7ykQzdlFb6hBoMqkCh0L9LNFSePPC6QctcitDd8VtTOy1eRPZPMO1afwSCM3q6LyjaF Dc1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772660267; x=1773265067; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=H3B6twlxs7RG5L5HQ0RR3EQNIcTaBDOh7BWXSLk64tU=; b=W3HILY8dlrJbDTru8Mp6xzgbVXRdgXOeSXTxFHS4J0jOsrh0cSpOf8GCP+8BJy82ks XgnVaDo+oi0achBsF++3VaPH6Hmy+3YpkCtusnOaPeyPvAEr6K1NrroB7L1f6phEl5Dh jtAJtMIQL2CV0kqwe6RATvEPNu/eztfv1i+cFBwPqemeAdJ7+GTC9QC2QhMsJmno20RD vhSnTIR1bFfs94/OIcX97BXox/bqOquq2Qw8VSbGrBCKLuEJTD6UpU0Xgaqo772RKbVg UJboDDZk95Q89fPFuLs/wS4xoJWIePpfclOw7CA8u3Q3q5WxjskQmmi5pd49nLLETrvm kDcQ== X-Forwarded-Encrypted: i=1; AJvYcCVjUnuxApLX0fMTKPSDDEUAEexAo2K53+eY6CCx2WK2G8VYKsbjrqOdYJeOLWfZBlUIUYc3ZY6xYk0iQaY=@vger.kernel.org X-Gm-Message-State: AOJu0YykdWUOjA3RErQV54LsJ03WDpMLYneUZnsBiCLYSXNxKUWvBTmq xo5D/2e7/fAbwwhEZtpsScIxv/EkraaPSLiiE5jRIcm2aYH1o0slXhldX9XFXubMrolRHgxpdf7 PZJBUcaGg2TNQnPmDHg== X-Received: from wmjx23.prod.google.com ([2002:a05:600c:21d7:b0:482:ef67:407f]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:a087:b0:485:110f:5b7f with SMTP id 5b1f17b1804b1-48519888e6amr59392995e9.19.1772660266742; Wed, 04 Mar 2026 13:37:46 -0800 (PST) Date: Wed, 4 Mar 2026 21:37:45 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260213-upgrade-poll-v2-0-984a0fb184fb@google.com> <20260213-upgrade-poll-v2-1-984a0fb184fb@google.com> Message-ID: Subject: Re: [PATCH v2 1/2] rust: poll: make PollCondVar upgradable From: Alice Ryhl To: Boqun Feng Cc: Christian Brauner , Boqun Feng , "Paul E. McKenney" , Gary Guo , Greg Kroah-Hartman , Carlos Llamas , linux-fsdevel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" On Wed, Mar 04, 2026 at 08:29:12AM -0800, Boqun Feng wrote: > On Wed, Mar 04, 2026 at 07:59:59AM +0000, Alice Ryhl wrote: > [...] > > > > + // If a normal waiter registers in parallel with us, then either: > > > > + // * We took the lock first. In that case, the waiter sees the above cmpxchg. > > > > + // * They took the lock first. In that case, we wake them up below. > > > > + drop(lock.lock()); > > > > + self.simple.notify_all(); > > > > > > Hmm.. what if the waiter gets its `&CondVar` before `upgrade()` and use > > > that directly? > > > > > > > > > let poll_cv: &UpgradePollCondVar = ...; > > > let cv = poll_cv.deref(); > > > cmpxchg(); > > > drop(lock.lock()); > > > self.simple.notify_all(); > > > let mut guard = lock.lock(); > > > cv.wait(&mut guard); > > > > > > we still miss the wake-up, right? > > > > > > It's creative, but I particularly hate we use an empty lock critical > > > section to synchronize ;-) > > > > I guess instead of exposing Deref, I can just implement `wait` directly > > on `UpgradePollCondVar`. Then this API misuse is not possible. > > > > If we do that,then we can avoid the `drop(lock.lock())` as well, if we > do: > > impl UpgradePollCondVar { > pub fn wait(...) { > prepare_to_wait_exclusive(); // <- this will take lock in > // simple.wait_queue_head. So > // either upgrade() comes > // first, or they observe the > // wait being queued. > let cv_ptr = self.active.load(Relaxed); > if !ptr_eq(cv_ptr, &self.simple) { // We have moved from > // simple, so need to > // need to wake up and > // redo the wait. > finish_wait(); > } else { > guard.do_unlock(|| { schedule_timeout(); }); > finish_wait(); > } > } > } > > (CondVar::notify*() will take the wait_queue_head lock as well) Yeah but then I have to actually re-implement those methods and not just call them. Seems not worth it. > > > Do you think the complexity of a dynamic upgrading is worthwhile, or we > > > should just use the box-allocated PollCondVar unconditionally? > > > > > > I think if the current users won't benefit from the dynamic upgrading > > > then we can avoid the complexity. We can always add it back later. > > > Thoughts? > > > > I do actually think it's worthwhile to consider: > > > > I started an Android device running this. It created 3961 instances of > > `UpgradePollCondVar` during the hour it ran, but only 5 were upgraded. > > > > That makes sense, thank you for providing the data! But still I think we > should be more informative about the performance difference between > dynamic upgrading vs. unconditionally box-allocated PollCondVar, because > I would assume when a `UpgradePollCondVar` is created, other allocations > also happen as well (e.g. when creating a Arc), so the > extra cost of the allocation may be unnoticeable. Perf-wise it doesn't matter, but I'm concerned about memory usage. Alice