From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D4E90CFB43F for ; Mon, 7 Oct 2024 10:40:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 494236B008A; Mon, 7 Oct 2024 06:40:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F5806B00AA; Mon, 7 Oct 2024 06:40:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26F426B009D; Mon, 7 Oct 2024 06:40:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 05C3D6B00E9 for ; Mon, 7 Oct 2024 06:40:55 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A3EB241675 for ; Mon, 7 Oct 2024 10:40:55 +0000 (UTC) X-FDA: 82646463270.05.49D3BAD Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) by imf06.hostedemail.com (Postfix) with ESMTP id 9B2BF18000A for ; Mon, 7 Oct 2024 10:40:51 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=f+wT0dE8; dmarc=none; spf=none (imf06.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728297611; a=rsa-sha256; cv=none; b=I8ETHKfvVmV7LNcOeDAG4MAlIFAY136ZbpFfmiiZc115SE9h6fQvUYgPQgWZ5KYX5judl1 qupsITUJwP1EzQin8mg9MUnHe55hienNQpyd3Unssqbe+BkyWVkalSKWBkwu0R8RnjSLGW p8IcoDx0GIonubQAlJTiCpxVt5tNV18= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=f+wT0dE8; dmarc=none; spf=none (imf06.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728297611; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=lwh9tJ35d1c8o+PW9Ww4chBCSD6YYk59hwSaIamIuE4=; b=vzTObhPiy/fiZIsXDmcHMO1i300LvBT4uSudrhCmosn4Q8B9sMSbwCZbnlTYeRkB61bRls FKlxzP59iF3bsXrSP2OfCiqxDuZyL0n3dKm6rJ46WHtsW3kqdSWlu1oe/IIbP4D5AGOvT2 FA/26BwXsHqiKLS76pYWwlDQhXKrb7Q= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=lwh9tJ35d1c8o+PW9Ww4chBCSD6YYk59hwSaIamIuE4=; b=f+wT0dE8i5aifk3S8szrr3YCCa y1323ZwpG++SyQKdW1lRpczCWQ+77ZXz9kiaoW1v+kOw6peFjjIumnq2ey2wgKdxCI+f6WtrSp3x4 HuYKDUuLrErt6hd93izRkawloew7tv6GMpCrWFB9N4FlV7LRsogofTLTpvId/Vwsk4VW9FiTJdIdf AJEPkx6FxaaVoH/bUEfBTqT/osHOtK8IVtIdfs1MDSmQH22gv6GPY84E0IBvCcCauya3KUjdujBtO mgynWr5+dLjvOz7bdkgsU6ZjCqFsp/rzp5hBijTJaCdjwShwM8kbMVf5xtnd2zD2zCidGORjpsEi3 kEocGheQ==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.98 #2 (Red Hat Linux)) id 1sxlAG-00000004PAP-02aA; Mon, 07 Oct 2024 10:40:32 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id D4DF030088D; Mon, 7 Oct 2024 12:40:30 +0200 (CEST) Date: Mon, 7 Oct 2024 12:40:30 +0200 From: Peter Zijlstra To: Mathieu Desnoyers Cc: Boqun Feng , linux-kernel@vger.kernel.org, Linus Torvalds , Andrew Morton , Nicholas Piggin , Michael Ellerman , Greg Kroah-Hartman , Sebastian Andrzej Siewior , "Paul E. McKenney" , Will Deacon , Alan Stern , John Stultz , Neeraj Upadhyay , Frederic Weisbecker , Joel Fernandes , Josh Triplett , Uladzislau Rezki , Steven Rostedt , Lai Jiangshan , Zqiang , Ingo Molnar , Waiman Long , Mark Rutland , Thomas Gleixner , Vlastimil Babka , maged.michael@gmail.com, Mateusz Guzik , Jonas Oberhauser , rcu@vger.kernel.org, linux-mm@kvack.org, lkmm@lists.linux.dev Subject: Re: [RFC PATCH v2 3/4] hp: Implement Hazard Pointers Message-ID: <20241007104030.GB4879@noisy.programming.kicks-ass.net> References: <20241004182734.1761555-1-mathieu.desnoyers@efficios.com> <20241004182734.1761555-4-mathieu.desnoyers@efficios.com> <20241005160444.GA18071@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: 9B2BF18000A X-Rspamd-Server: rspam01 X-Stat-Signature: dzc7crumtc8e7nh6yqh6ispze5zmzb6i X-HE-Tag: 1728297651-611382 X-HE-Meta: U2FsdGVkX18DRye5K6CgpiVb3w3tfyjYhSS4GhRYROq0ziBr/9c6fcvMAjDLBYARoHy4mJNqXHu94GUzHyImMyeRydqR3hdnbL8J4o11NUEHWKqHvUolU/YManK1syMIY/u5vfWyRDo5G1bRzMhQUQ2NEKcfx83b9aVii1jKjwhlitZBz7TXFpWY3adky3X6FEK4ZimpQHNBCWZyk57Z4vfy3JnhL9J4ccwEpn58nwE9dhMGHKxrA+ndqgy448ZTgg80uxUFm+XIlOBmgQfSKEzQmxRFLLQBpY0aney3YMFfL9DumUTAQ6RfbJfHVK75ogqR3qoAnvaD3LLkZ/ND23WSBU6TBFcjzYE7UrnGHQNgAPvCO48QgLCcJh9Js5X9xwfRwxFQRTo9j9ULLV1TLJTWujhPCFUDWVASUt3NKeSh++ewHgWlHLY7yAWBRzbfNLbepQ8Y7znvdsSu6vcgLTW6T5Wlz4BOeAIn3xo8aiD5ueNuo7AtvRFeNR72/sgsnCxVGXjAbVVn+ABNAveEnCcwUR1W5MruaZ1SmqN1gdqeJ0hMkTYGNmN3qoUBikYSexphSQiOjFAw3SAtPooBtqLne5glAwOlWxJ1g+uKlUujuLnwdUxquTAauGnegioh/0FkaFWDwTZ8ck8u00X2j6AjBOOVW/XC4GTyWRlyD7tC5/vwAOCUNTLYFpADdnQcwngYFTEM9xVf4SLNz24ITC/ifHY9rf6MdnRCc0S8GNT3r6aLs3k+5laypxbqVxU5pfyw/vIsWktbTiAknBRhBfXt5SZya5zUCZJIB2Xcz3qYAXewgsMpEgTp7LsoIjZ68QSKieYgKBRCwl4ksvXRp/Prs6ghfG7IlBssqR8d+6h0L6M1HehxKqaXX3h6a3ohafH898s+zs9F6sMwrm+yuBlabDcYMp1sTx9ZlZwM+5vYyj9CgDChMtmbBvXcD2nGY9AnhaBDopaHrctfi4C BfrA29Ri D6/EyulUbVC4K9Fup9WzqcJFZgUlue5lX/1okn9HX+wFBIiwHu+9FCXuxgwuc93DAOqiUsEYBuPDEiA2HO973XK1xIMARfSqS0y2p2r4MV2Zg3xPejMZ99ZPGtYxGP4YmnYew3JsRBhgCXVaFeYthxXoYq149g5MCgEPX6KCjljU5Jk3JevpzZCR9W/1QMzXUe1ejg/N8U2Vaffsy0q82HBubMcrCXuuXMQv7f30wanEecbLC/qXxJzOF2s6TxT3vrka0c5C+dE7562M= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Oct 05, 2024 at 02:50:17PM -0400, Mathieu Desnoyers wrote: > On 2024-10-05 18:04, Peter Zijlstra wrote: > > > +/* > > > + * hp_allocate: Allocate a hazard pointer. > > > + * > > > + * Allocate a hazard pointer slot for @addr. The object existence should > > > + * be guaranteed by the caller. Expects to be called from preempt > > > + * disable context. > > > + * > > > + * Returns a hazard pointer context. > > > > So you made the WTF'o'meter crack, this here function does not allocate > > nothing. Naming is bad. At best this is something like > > try-set-hazard-pointer or somesuch. > > I went with the naming from the 2004 paper from Maged Michael, but I > agree it could be clearer. > > I'm tempted to go for "hp_try_post()" and "hp_remove()", basically > "posting" the intent to use a pointer (as in on a metaphorical billboard), > and removing it when it's done. For RCU we've taken to using the word: 'publish', no? > > > +/* > > > + * hp_dereference_allocate: Dereference and allocate a hazard pointer. > > > + * > > > + * Returns a hazard pointer context. Expects to be called from preempt > > > + * disable context. > > > + */ > > > > More terrible naming. Same as above, but additionally, I would expect a > > 'dereference' to actually dereference the pointer and have a return > > value of the dereferenced type. > > hp_dereference_try_post() ? > > > > > This function seems to double check and update the hp_ctx thing. I'm not > > at all sure yet wtf this is doing -- and the total lack of comments > > aren't helping. > > The hp_ctx contains the outputs. > > The function loads *addr_p to then try_post it into a HP slot. On success, > it re-reads the *addr_p (with address dependency) and if it still matches, > use that as output address pointer. > > I'm planning to remove hp_ctx, and just have: > > /* > * hp_try_post: Try to post a hazard pointer. > * > * Post a hazard pointer slot for @addr. The object existence should > * be guaranteed by the caller. Expects to be called from preempt > * disable context. > * > * Returns true if post succeeds, false otherwise. > */ > static inline > bool hp_try_post(struct hp_domain *hp_domain, void *addr, struct hp_slot **_slot) > [...] > > /* > * hp_dereference_try_post: Dereference and try to post a hazard pointer. > * > * Returns a hazard pointer context. Expects to be called from preempt > * disable context. > */ > static inline > void *__hp_dereference_try_post(struct hp_domain *hp_domain, > void * const * addr_p, struct hp_slot **_slot) > [...] > > #define hp_dereference_try_post(domain, p, slot_p) \ > ((__typeof__(*(p))) __hp_dereference_try_post(domain, (void * const *) p, slot_p)) This will compile, but do the wrong thing when p is a regular pointer, no? > > /* Clear the hazard pointer in @slot. */ > static inline > void hp_remove(struct hp_slot *slot) > [...] Differently weird, but better I suppose :-) > > > +void hp_scan(struct hp_slot __percpu *percpu_slots, void *addr, > > > + void (*retire_cb)(int cpu, struct hp_slot *slot, void *addr)) > > > +{ > > > + int cpu; > > > + > > > + /* > > > + * Store A precedes hp_scan(): it unpublishes addr (sets it to > > > + * NULL or to a different value), and thus hides it from hazard > > > + * pointer readers. > > > + */ > > > + > > > + if (!addr) > > > + return; > > > + /* Memory ordering: Store A before Load B. */ > > > + smp_mb(); > > > + /* Scan all CPUs slots. */ > > > + for_each_possible_cpu(cpu) { > > > + struct hp_slot *slot = per_cpu_ptr(percpu_slots, cpu); > > > + > > > + if (retire_cb && smp_load_acquire(&slot->addr) == addr) /* Load B */ > > > + retire_cb(cpu, slot, addr); > > > > Is retirce_cb allowed to cmpxchg the thing? > > It could, but we'd need to make sure the slot is not re-used by another > hp_try_post() before the current user removes its own post. It would > need to synchronize with the current HP user (e.g. with IPI). > > I've actually renamed retire_cb to "on_match_cb". Hmm, I think I see. Would it make sense to pass the expected addr to hp_remove() and double check we don't NULL out something unexpected? -- maybe just for a DEBUG option. I'm always seeing the NOHZ_FULL guys hating on this :-)