From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D13DB188736 for ; Sun, 22 Dec 2024 11:36:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734867373; cv=none; b=d5i/EN/ZHLLbl8MbHtVBUSGYRNJnCyl5Sz3dE3orvjpoaqkSL8lem4/lpbl87G6QzPr68IMpcIsfEZnzKNsb9YahxaWM2vWTDsI5IIHfYNXHTu69eoYNUTRvMfYObBRrD5SJK1XnXGu12jzKO9EhmsUfeRPml/3dRmSYmIcfK8Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734867373; c=relaxed/simple; bh=kYlRjo4y0WH86qFcq+J+Lz1JRsu99loLVVyiTfFDM2w=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Bm9qhQLdvf+37qQatRCntjL1GQupis/bLJQ6l/DlNN+zkl17AZUjxKJiLZPcIcg7zMzHFLQpfIFODPU+sQ5q7GR97ETls9NaW4CQVAME/3LV2QyVLlPmWdzUGaXTrFDKQecsWyyeaYWH2aZpAM4DRJw/iVFc+lwPO79V6e9ws0k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=LCgiPV4Z; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="LCgiPV4Z" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; 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=wkILQH3Yez/hdb1RR7PWT30Uzw/34PVg6iwDiEUXL0I=; b=LCgiPV4ZL/WUCijkQIRDTxDw1j 9lDhkin0ZU6AAtqMSgaHwO3oNstb5k8GMLrJBs5TnoFQz5pnZUKPNnBa5ILjpnc9d+kSS99IIuRG0 NBzavN7OcmED8O/Sqp9VOHL/D0UanSbpgE1aHOL/yyFbpgwykPVxhUYHTdQjN5S4vMJn3NqyMVwjc tOC6wZGzM/5l2LZXMh33YKbAtaiKxR76pAdedtyK8pS03FbN9nSBvysRt+rSzhQ22yGee1CZHENha oUahA6s7ub00FHZlFOR/XOF9sUJX1vStDvL0IcX0x6wdA9nc4tnRIR82NyZPKtHMGhp/3OeqOQuDM +R3MDXhQ==; Received: from 77-249-17-89.cable.dynamic.v4.ziggo.nl ([77.249.17.89] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.98 #2 (Red Hat Linux)) id 1tPKFd-0000000AWh9-2Dmv; Sun, 22 Dec 2024 11:36:01 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 572F53003AF; Sun, 22 Dec 2024 12:36:01 +0100 (CET) Date: Sun, 22 Dec 2024 12:36:01 +0100 From: Peter Zijlstra To: Rik van Riel Cc: x86@kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com, dave.hansen@linux.intel.com, luto@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com, akpm@linux-foundation.org Subject: Re: [PATCH 07/10] x86,mm: enable broadcast TLB invalidation for multi-threaded processes Message-ID: <20241222113601.GX11133@noisy.programming.kicks-ass.net> References: <20241222040717.3096835-1-riel@surriel.com> <20241222040717.3096835-8-riel@surriel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241222040717.3096835-8-riel@surriel.com> On Sat, Dec 21, 2024 at 11:06:39PM -0500, Rik van Riel wrote: > +#ifdef CONFIG_CPU_SUP_AMD > +/* > + * Logic for AMD INVLPGB support. > + */ > +static DEFINE_SPINLOCK(broadcast_asid_lock); RAW_SPINLOCK ? > +static u16 last_broadcast_asid = TLB_NR_DYN_ASIDS; > +static DECLARE_BITMAP(broadcast_asid_used, MAX_ASID_AVAILABLE) = { 0 }; > +static LIST_HEAD(broadcast_asid_list); > +static int broadcast_asid_available = MAX_ASID_AVAILABLE - TLB_NR_DYN_ASIDS - 1; > + > +static void reset_broadcast_asid_space(void) > +{ > + mm_context_t *context; > + > + assert_spin_locked(&broadcast_asid_lock); lockdep_assert_locked(&broadcast_asid_lock); > + > + /* > + * Flush once when we wrap around the ASID space, so we won't need > + * to flush every time we allocate an ASID for boradcast flushing. > + */ > + invlpgb_flush_all_nonglobals(); > + tlbsync(); > + > + /* > + * Leave the currently used broadcast ASIDs set in the bitmap, since > + * those cannot be reused before the next wraparound and flush.. > + */ > + bitmap_clear(broadcast_asid_used, 0, MAX_ASID_AVAILABLE); > + list_for_each_entry(context, &broadcast_asid_list, broadcast_asid_list) > + __set_bit(context->broadcast_asid, broadcast_asid_used); > + > + last_broadcast_asid = TLB_NR_DYN_ASIDS; > +} > + > +static u16 get_broadcast_asid(void) > +{ > + assert_spin_locked(&broadcast_asid_lock); lockdep_assert_locked > + > + do { > + u16 start = last_broadcast_asid; > + u16 asid = find_next_zero_bit(broadcast_asid_used, MAX_ASID_AVAILABLE, start); > + > + if (asid >= MAX_ASID_AVAILABLE) { > + reset_broadcast_asid_space(); > + continue; > + } > + > + /* Try claiming this broadcast ASID. */ > + if (!test_and_set_bit(asid, broadcast_asid_used)) { > + last_broadcast_asid = asid; > + return asid; > + } > + } while (1); > +} > + > +/* > + * Returns true if the mm is transitioning from a CPU-local ASID to a broadcast > + * (INVLPGB) ASID, or the other way around. > + */ > +static bool needs_broadcast_asid_reload(struct mm_struct *next, u16 prev_asid) > +{ > + u16 broadcast_asid = next->context.broadcast_asid; > + > + if (broadcast_asid && prev_asid != broadcast_asid) { > + return true; > + } > + > + if (!broadcast_asid && is_broadcast_asid(prev_asid)) { > + return true; > + } > + > + return false; > +} Those return statements don't really need {} on. > + > +void destroy_context_free_broadcast_asid(struct mm_struct *mm) { { goes on a new line. > + unsigned long flags; > + > + if (!mm->context.broadcast_asid) > + return; > + > + spin_lock_irqsave(&broadcast_asid_lock, flags); guard(raw_spin_lock_irqsave)(&broadcast_asid_lock); > + mm->context.broadcast_asid = 0; > + list_del(&mm->context.broadcast_asid_list); > + broadcast_asid_available++; > + spin_unlock_irqrestore(&broadcast_asid_lock, flags); > +} > + > +static int mm_active_cpus(struct mm_struct *mm) > +{ > + int count = 0; > + int cpu; > + > + for_each_cpu(cpu, mm_cpumask(mm)) { > + /* Skip the CPUs that aren't really running this process. */ > + if (per_cpu(cpu_tlbstate.loaded_mm, cpu) != mm) > + continue; > + > + if (per_cpu(cpu_tlbstate_shared.is_lazy, cpu)) > + continue; > + > + count++; > + } > + return count; > +} > + > +/* > + * Assign a broadcast ASID to the current process, protecting against > + * races between multiple threads in the process. > + */ > +static void use_broadcast_asid(struct mm_struct *mm) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&broadcast_asid_lock, flags); guard(raw_spin_lock_irqsave)(&broadcast_asid_lock); > + > + /* This process is already using broadcast TLB invalidation. */ > + if (mm->context.broadcast_asid) > + goto out_unlock; return; > + mm->context.broadcast_asid = get_broadcast_asid(); > + mm->context.asid_transition = true; > + list_add(&mm->context.broadcast_asid_list, &broadcast_asid_list); > + broadcast_asid_available--; > + > +out_unlock: Notably we're really wanting to get away from the whole goto unlock pattern. > + spin_unlock_irqrestore(&broadcast_asid_lock, flags); > +} > + > +/* > + * Figure out whether to assign a broadcast (global) ASID to a process. > + * We vary the threshold by how empty or full broadcast ASID space is. > + * 1/4 full: >= 4 active threads > + * 1/2 full: >= 8 active threads > + * 3/4 full: >= 16 active threads > + * 7/8 full: >= 32 active threads > + * etc > + * > + * This way we should never exhaust the broadcast ASID space, even on very > + * large systems, and the processes with the largest number of active > + * threads should be able to use broadcast TLB invalidation. I'm a little confused, at most we need one ASID per CPU, IIRC we have something like 4k ASIDs (page-offset bits in the physical address bits) so for anything with less than 4K CPUs we're good, but with anything having more CPUs we're up a creek irrespective of the above scheme, no? > + */ > +#define HALFFULL_THRESHOLD 8 > +static bool meets_broadcast_asid_threshold(struct mm_struct *mm) > +{ > + int avail = broadcast_asid_available; > + int threshold = HALFFULL_THRESHOLD; > + int mm_active_threads; > + > + if (!avail) > + return false; > + > + mm_active_threads = mm_active_cpus(mm); > + > + /* Small processes can just use IPI TLB flushing. */ > + if (mm_active_threads < 3) > + return false; > + > + if (avail > MAX_ASID_AVAILABLE * 3 / 4) { > + threshold = HALFFULL_THRESHOLD / 4; > + } else if (avail > MAX_ASID_AVAILABLE / 2) { > + threshold = HALFFULL_THRESHOLD / 2; > + } else if (avail < MAX_ASID_AVAILABLE / 3) { > + do { > + avail *= 2; > + threshold *= 2; > + } while ((avail + threshold ) < MAX_ASID_AVAILABLE / 2); > + } > + > + return mm_active_threads > threshold; > +}