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 33B5DC001DE for ; Mon, 31 Jul 2023 21:34:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AA0BB2800AC; Mon, 31 Jul 2023 17:34:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A516528007A; Mon, 31 Jul 2023 17:34:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 942D92800AC; Mon, 31 Jul 2023 17:34:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 87DC028007A for ; Mon, 31 Jul 2023 17:34:00 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 55925A03F7 for ; Mon, 31 Jul 2023 21:34:00 +0000 (UTC) X-FDA: 81073209840.20.DB1A623 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) by imf06.hostedemail.com (Postfix) with ESMTP id 78C56180009 for ; Mon, 31 Jul 2023 21:33:57 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=cGwctQdt; 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=1690839238; 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=wgnR5bAqL2xpttjtPlBIH4TwsFtkouzs/SF7LfIw4XQ=; b=eeymjDL3DX42UZ99byQu9MImC0yA+OGUlNc8pyE+OiZ1uVihiTxaCiDNS5Oy3WTBVKpFs9 GcB1exiQtNrt3UhIK1tAj44tp2V9DuES7Y7IHL2xoqwtbVcGzvpfBe/tK3dOABVQqLph7M l6/Qgx4aCan59VpGHcZZ57CUf/7WYBE= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=cGwctQdt; 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=1690839238; a=rsa-sha256; cv=none; b=N85x8Laaw3NJ3w2dkjaYNmLHpTEfCyjxDsTb4HbKl+Aio3aSFDM2MHy2082Wh0qSAxqhXB 32D1n0nbTX08QV0nAcjttnvtH800pjYV3zIv9WbM9b+l3vy+IOWlcypxER7D5YOi1kgIkT nCFI+Ne1hD28w5RCEu3UHqCabfXGS2M= 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=wgnR5bAqL2xpttjtPlBIH4TwsFtkouzs/SF7LfIw4XQ=; b=cGwctQdtUKzHZtGCTBUljHHpAF Gyl9ukMe9e3MBczirXiF1/uR4DvnO/D84pT8HNnCnkT48x4TW4nczA9lFALEOxUwLo1oq/1o5CQO7 QzRi715jiHWfqwdsYA3zmCyzUpBStsy4ATFsOl82DLdq7xbzyIPfxy6pOI8Kf5LPeFPrqpQnROW5H 9LGnhvTkARlC1P44faAzLBGpnIUTvTd2GQbmU/KCD4zVa+ziRperTNnNINfcDtk3UIhe8AbjsHAkq tsTTNwTojYd2oIL3msBz2uFrPDNJR0kIOX7yFddyHevyYN4Zt56jZB27bAs6E7Gw/zy0cqgm94lUX Vf7M7X6Q==; 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.96 #2 (Red Hat Linux)) id 1qQaWM-00D5kz-0W; Mon, 31 Jul 2023 21:33:42 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 50B143002CE; Mon, 31 Jul 2023 23:33:41 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 1F5EB213D004E; Mon, 31 Jul 2023 23:33:41 +0200 (CEST) Date: Mon, 31 Jul 2023 23:33:41 +0200 From: Peter Zijlstra To: Thomas Gleixner Cc: axboe@kernel.dk, linux-kernel@vger.kernel.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 02/14] futex: Extend the FUTEX2 flags Message-ID: <20230731213341.GB51835@hirez.programming.kicks-ass.net> References: <20230721102237.268073801@infradead.org> <20230721105743.819362688@infradead.org> <87edkonjrk.ffs@tglx> <87mszcm0zw.ffs@tglx> <20230731192012.GA11704@hirez.programming.kicks-ass.net> <87a5vbn5r0.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a5vbn5r0.ffs@tglx> X-Rspamd-Queue-Id: 78C56180009 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: x4x7xu4y88kfrh5jqxjdqsbbzxw7ck4z X-HE-Tag: 1690839237-626716 X-HE-Meta: U2FsdGVkX19FjW9mr2LMRzlVQ6QjPkwq7utYx/aKN5wxNZmqrcvHqKs3+g6eEqVaI0MbS0H0QxkGE8jk998EgSAX56FLgoCZxDKQnPyhu5U7DitR74Leq0OwiJim+BaQavBU07hy1iqDHbXkaRwqN/tXn5/qeDyQ7Hwy5u90XCDY0/AQG09VJ9woPaoXnWxczUpmvQtL86UtCujGaMVHE851CvmHNEK5dZfRcVhHGl2Kl5GwJd937am1xx834ghGXD6B0yJX4jjwSG2Q63GnWAlkA/cbeaSeo+8yQ0bXl71GUrweGgn0yFd0WXKUcDwQ+euxoeZsgCXOVK93nggBtDlCop33MTRdhztyKB1Jk1hFqLvIxXz9PlnmjL9kc9JANwbMdBt2x99fT+q3tHI9OOaoru+gKYNJvgF6nTGbK/nMnnggdrzicaHXtWeXydUFfgYujCyGn7iQePuMLs4UcmVuABuxaVmAnN9a3wE7XZj5cTaqClb47Z0pdT2e45u4M+EgNoRXt09BI/A8nJDxpKkIYD6lQ9EL5q6yMGUXe6E+lf7BNR9ssHQBX4/UDmL96JMFNaC9rCYuzq10Yw2UiDhOrsN3ZSlml5G0A2u5GwDJX7P5CmdpLTJ5oZpSndGwgkAj//tP+RdgWS1VSMlNNPwj4eVLY65fklw4DN9JwzmrjGtIat8Cm5kpIKP0CG/bwJbyfKXyKwqsRXqgGSQ3Mk9l5/V6WqpR41Af3Q6btJW69tNc/4IBDdmLP0h3pd1VCbm//a7MtUGgZZm6mad9QtFaExHD+yDJJgqPB0CojSjj67+pbh5w03xkFWJymCxPFVO5D69A5mBAunurmEE89e0ZDi5SDAwmBcSC5TNv+gGLnBvqGnHOOEZih+If6GLdrBpK46/NReYoS4Bh91RtZeBWnv9JtOQwCG3XCsXa5GGU8pXmY0LFTbltTC6s4G0+Qyl6gTNPMxgJAvjhPQM qrl2RatZ jBBbwIp/js3iHNV58NK3PM2bS1IbjmVQoJY++KRPyowHqjYlToFblSO5p7dkl08IbTZ2Fji44Y4qGUBru8FhuzpXKgM8Rdzof2umw52X3HafgVS+xi7gJ185ZQo+e+b+/kHTB2TK0zlbqauM= 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 Mon, Jul 31, 2023 at 11:14:11PM +0200, Thomas Gleixner wrote: > On Mon, Jul 31 2023 at 21:20, Peter Zijlstra wrote: > > -#define FUTEX2_MASK (FUTEX2_64 | FUTEX2_PRIVATE) > > +#define FUTEX2_MASK (FUTEX2_SIZE_MASK | FUTEX2_PRIVATE) > > Along with some comment which documents that the size "flags" constitute > a number field and not flags in the sense of binary flags. > > And please name these size constants so it really becomes obvious: > > #define FUTEX2_SIZE_U32 2 So you want them named: #define FUTEX2_SIZE_U8 0x00 #define FUTEX2_SIZE_U16 0x01 #define FUTEX2_SIZE_U32 0x02 #define FUTEX2_SIZE_U64 0x03 #define FUTEX2_SIZE_MASK 0x03 Sure, can do. > > /** > > * futex_parse_waitv - Parse a waitv array from userspace > > @@ -208,11 +208,11 @@ static int futex_parse_waitv(struct fute > > return -EINVAL; > > > > if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall()) { > > - if ((aux.flags & FUTEX2_64) == FUTEX2_64) > > + if ((aux.flags & FUTEX2_SIZE_MASK) == FUTEX2_64) > > return -EINVAL; > > } > > That should be part of the actual 64bit futex enablement, no? The 'unsigned long' thing is part of the syscalls, which is why I had it now. > > > - if ((aux.flags & FUTEX2_64) != FUTEX2_32) > > + if ((aux.flags & FUTEX2_SIZE_MASK) != FUTEX2_32) > > return -EINVAL; > > In hindsight I think it was as mistake just to have this __u32 flags > field in the new interface. Soemthing like the incomplete below might be > retrofittable, no? > > --- a/include/uapi/linux/futex.h > +++ b/include/uapi/linux/futex.h > @@ -74,7 +74,12 @@ > struct futex_waitv { > __u64 val; > __u64 uaddr; > - __u32 flags; > + union { > + __u32 flags; > + __u32 size : 2, > + : 5, > + private : 1; > + }; > __u32 __reserved; > }; Durr, I'm not sure I remember if that does the right thing across architectures -- might just work. But I'm fairly sure this isn't the only case of a field in a flags thing in our APIs. Although obviously I can't find another case in a hurry :/ Also, sys_futex_{wake,wait}() have this thing as a syscall argument, surely you don't want to put this union there as well? I'd much prefer to just keep the 'unsigned int flags' thing and perhaps put a comment on-top of the '#define FUTEX2_*' thingies. Note that having it a field instead of a bunch of flags makes sense, since you can only have a single size, not a combination of sizes.