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 D5745C001DE for ; Mon, 31 Jul 2023 17:36:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6455828008F; Mon, 31 Jul 2023 13:36:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5F57028007A; Mon, 31 Jul 2023 13:36:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4E4CF28008F; Mon, 31 Jul 2023 13:36:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 3F13C28007A for ; Mon, 31 Jul 2023 13:36:26 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 0F220A032D for ; Mon, 31 Jul 2023 17:36:26 +0000 (UTC) X-FDA: 81072611172.13.606B656 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf24.hostedemail.com (Postfix) with ESMTP id 3C23218001A for ; Mon, 31 Jul 2023 17:36:23 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=sniaLJjU; dkim=pass header.d=linutronix.de header.s=2020e header.b=Cw3umMTe; spf=pass (imf24.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690824984; 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=iFf50TJmeKBJp8QFVacRBeJgY/I84fMWpugiLSkN2d0=; b=OgYuqDwFjzqQj7wl8BErt6QmxQ1A6JYbX4P2SxAFgsb3waxZpsLW7ZBVTNDkBNAQwodeVj L8ZAw1pl+8520P35e5VLg6Vg5BOZJw0KFUs3s7EtQUL1C7b4xFXwBrS/1P2L4S2gDaQv6O 9axGUPPk7pdSKfTXAisACrjPy3x2iP0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690824984; a=rsa-sha256; cv=none; b=sf7kwAFTAXyBAYbnUf8ServM1/EJR8qE46Hh/swLfeiEPtIUSQwYAJILJJtECdbOhlgMoB 89dH6xt8QpJ/YUNwHvEUAsd2Xxi/0sAALmfJoE3q0bSkuLzO/VWXxahV2Z3fSNLssrm+VS imjhbpkKcWxFf89I2BMk43tbj9BjVnM= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=sniaLJjU; dkim=pass header.d=linutronix.de header.s=2020e header.b=Cw3umMTe; spf=pass (imf24.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1690824982; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=iFf50TJmeKBJp8QFVacRBeJgY/I84fMWpugiLSkN2d0=; b=sniaLJjU9khAD/zHKPICJex4h/W1UTe8KR4cp/RPdiVDPzRAFxo6US8Cdk2noeDT7T75WD xFGgh+YZpmXu+CC6kd+8wl+oqCWe+vfe5w/f8+8RDJtb9ut+PI2ZCtSZ1VHR4ZSvMMknvb CABTIObaYdIIbbS8TwAZ83yTuujof1JjdtF7Q8zFdVl3Bu32mOgEmhg9xoVhXFomk54AuQ gEQk1K3riDtf5IYeLNtUcAHDz3djZFzJFtGf/3P7Dln4zK0+fdepJEpmreCcQ+/Po50hyy FwZ+im0VEcR6R1j6qTkTtkvxHF0eN7N6z+HmS6FA1Erlj15b9s+P8DOSJdyQrQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1690824982; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=iFf50TJmeKBJp8QFVacRBeJgY/I84fMWpugiLSkN2d0=; b=Cw3umMTe1MDbI7v/AYOO9l3z7Lq/XpIXoigLOz6Wy828nDvlximePUklxfgQRXPmJMrasA ENUcHEe3LhopAxBg== To: Peter Zijlstra , axboe@kernel.dk Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, dvhart@infradead.org, dave@stgolabs.net, andrealmeid@igalia.com, Andrew Morton , urezki@gmail.com, hch@infradead.org, lstoakes@gmail.com, Arnd Bergmann , linux-api@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, malteskarupke@web.de Subject: Re: [PATCH v1 11/14] futex: Implement FUTEX2_NUMA In-Reply-To: <20230721105744.434742902@infradead.org> References: <20230721102237.268073801@infradead.org> <20230721105744.434742902@infradead.org> Date: Mon, 31 Jul 2023 19:36:21 +0200 Message-ID: <87pm48m19m.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Queue-Id: 3C23218001A X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: xtmqwggkzgmg5jud4qk6qeqmer511cp9 X-HE-Tag: 1690824983-994686 X-HE-Meta: U2FsdGVkX18394uyuaPJ4MRHwwnJ++PFxe1KQsjK+LWR5edOO7VsgOfKyWms2UxmkglN2V2ZGvMFlWSxbDDhq9g3/v2HQCpKW7sBaYrp9rXz5/NrVcoY0p8gA+APoZA6Dn41cBrlZzy31FN3cz0A9yEUow7FXZQAI7ABFxgEfQ/uSwomIl/+nc9n/wKceA7wqHCYT9AeAS4tAWnSjMovBDBY89dl3ZzL6cKkWtxeKAT0YhUy0O3tapOSnjbSFXs5fnTZx1dom5WBEWgJOLrVf6uLn/D/ys8n+n+diOVJEzf23vZNiQ3BAMc9Hs0JFC6WoOmFjn34PjoEul9fvk6hEUKGiuuKGfY3Z/9WkrtUe4d0Wnlky6aIOFQm1r7EeCUg/jvSjgAFuqJZwUcb8Z4kM1j3WmgvHvjgJE3KsRNRzOT0cxmyZTcYwTMeuHefYKfxA5Jd7VexRbnWb4o5UigHDT5OPBDHo159Bfj0eH6Trj7xdMkNYJ+3wGyGNe9JrRysyfJDp1clqonndeXp1pUnSJ2mjCLnumKfF2apa8MwSd4U1jz0eLlCv0wU9S4Y707HF2Q+jedu1vtcdjq1dgKthdKaTgbA0sQi0QU4xaZ6dy3kT3IHdGjbP/EB/2Z0VWJiG/+bZQlFDHLY/Lm2iWnjsLe/MRtKbIrVtcITMXqcGLNYpXQHJdpg8vLXoItBfDNHp5SOKo1d6PcD0phueKpy8zwoBnVMNMv/Y1T/2mdYnVCIA3FjVhqotsdEqeTH9gCnQ4wcRdUtA3mS/TTR02twc6RwgJg2XhcUAUqj0OKu4eVRZEuGhe7aM8Bgn3TroEDjUY+lgxpdFsb7+phJ/d6EP6ks1/YPbivZmSFD0q96Ukl6PKR9Sl+VZqr6+LOlrx+GIUsZU/jicZDjsfxoV50WOxFQAVAvpfWtMwjeVGubZXlqISmjyOuJmp3fUN5vToNhFz1Wq4eHV4Z+5Aa71Ql 0EZ+nga0 NLuLuzyDo6N/AQykVkI2Jk6k7xOnGe1/4dAlLOFCMI2YVb0c8s4ypW5V45bN70e98xVCgV9k287+VqZU06Q5jdheFSVWMndg2JikK 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: On Fri, Jul 21 2023 at 12:22, Peter Zijlstra wrote: > struct futex_hash_bucket *futex_hash(union futex_key *key) > { > - u32 hash = jhash2((u32 *)key, offsetof(typeof(*key), both.offset) / 4, > + u32 hash = jhash2((u32 *)key, > + offsetof(typeof(*key), both.offset) / sizeof(u32), > key->both.offset); > + int node = key->both.node; > > - return &futex_queues[hash & (futex_hashsize - 1)]; > + if (node == -1) { > + /* > + * In case of !FLAGS_NUMA, use some unused hash bits to pick a > + * node -- this ensures regular futexes are interleaved across > + * the nodes and avoids having to allocate multiple > + * hash-tables. > + * > + * NOTE: this isn't perfectly uniform, but it is fast and > + * handles sparse node masks. > + */ > + node = (hash >> futex_hashshift) % nr_node_ids; Is nr_node_ids guaranteed to be stable after init? It's marked __read_mostly, but not __ro_after_init. > + if (!node_possible(node)) { > + node = find_next_bit_wrap(node_possible_map.bits, > + nr_node_ids, node); > + } > + } > + > + return &futex_queues[node][hash & (futex_hashsize - 1)]; > } > fshared = flags & FLAGS_SHARED; > + size = futex_size(flags); > > /* > * The futex address must be "naturally" aligned. > */ > key->both.offset = address % PAGE_SIZE; > - if (unlikely((address % sizeof(u32)) != 0)) > + if (unlikely((address % size) != 0)) > return -EINVAL; Hmm. Shouldn't that have changed with the allowance of the 1 and 2 byte futexes? > address -= key->both.offset; > > - if (unlikely(!access_ok(uaddr, sizeof(u32)))) > + if (flags & FLAGS_NUMA) > + size *= 2; > + > + if (unlikely(!access_ok(uaddr, size))) > return -EFAULT; > > if (unlikely(should_fail_futex(fshared))) > return -EFAULT; > > + key->both.node = -1; Please put this into an else path. > + if (flags & FLAGS_NUMA) { > + void __user *naddr = uaddr + size/2; size / 2; > + > + if (futex_get_value(&node, naddr, flags)) > + return -EFAULT; > + > + if (node == -1) { > + node = numa_node_id(); > + if (futex_put_value(node, naddr, flags)) > + return -EFAULT; > + } > + > + if (node >= MAX_NUMNODES || !node_possible(node)) > + return -EINVAL; That's clearly an else path too. No point in checking whether numa_node_id() is valid. > + key->both.node = node; > + } > > +static inline unsigned int futex_size(unsigned int flags) > +{ > + unsigned int size = flags & FLAGS_SIZE_MASK; > + return 1 << size; /* {0,1,2,3} -> {1,2,4,8} */ > +} > + > static inline bool futex_flags_valid(unsigned int flags) > { > /* Only 64bit futexes for 64bit code */ > @@ -77,13 +83,19 @@ static inline bool futex_flags_valid(uns > if ((flags & FLAGS_SIZE_MASK) != FLAGS_SIZE_32) > return false; > > - return true; > -} > + /* > + * Must be able to represent both NUMA_NO_NODE and every valid nodeid > + * in a futex word. > + */ > + if (flags & FLAGS_NUMA) { > + int bits = 8 * futex_size(flags); > + u64 max = ~0ULL; > + max >>= 64 - bits; Your newline key is broken, right? > + if (nr_node_ids >= max) > + return false; > + } Thanks, tglx