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 X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6324DC433F5 for ; Thu, 16 Sep 2021 04:10:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 480F86113E for ; Thu, 16 Sep 2021 04:10:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230170AbhIPELx convert rfc822-to-8bit (ORCPT ); Thu, 16 Sep 2021 00:11:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47950 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229463AbhIPELv (ORCPT ); Thu, 16 Sep 2021 00:11:51 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A0530C061574; Wed, 15 Sep 2021 21:10:31 -0700 (PDT) Received: from localhost (unknown [IPv6:2a00:5f00:102:0:f4d2:afff:fe2b:18b5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: krisman) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 161211F435AC; Thu, 16 Sep 2021 05:10:28 +0100 (BST) From: Gabriel Krisman Bertazi To: =?utf-8?Q?Andr=C3=A9?= Almeida Cc: Thomas Gleixner , Ingo Molnar , Peter Zijlstra , Darren Hart , linux-kernel@vger.kernel.org, Steven Rostedt , Sebastian Andrzej Siewior , kernel@collabora.com, linux-api@vger.kernel.org, libc-alpha@sourceware.org, mtk.manpages@gmail.com, Davidlohr Bueso , Arnd Bergmann Subject: Re: [PATCH v3 2/6] futex2: Implement vectorized wait Organization: Collabora References: <20210913175249.81074-1-andrealmeid@collabora.com> <20210913175249.81074-3-andrealmeid@collabora.com> <875yv4ge83.fsf@collabora.com> <58536544-e032-1954-ce30-d131869dc95e@collabora.com> Date: Thu, 16 Sep 2021 00:10:25 -0400 In-Reply-To: <58536544-e032-1954-ce30-d131869dc95e@collabora.com> (=?utf-8?Q?=22Andr=C3=A9?= Almeida"'s message of "Tue, 14 Sep 2021 14:18:58 -0300") Message-ID: <8735q5dutq.fsf@collabora.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org André Almeida writes: >>> +/** >>> + * struct futex_waitv - A waiter for vectorized wait >>> + * @val: Expected value at uaddr >>> + * @uaddr: User address to wait on >>> + * @flags: Flags for this waiter >>> + * @__reserved: Reserved member to preserve data alignment. Should be 0. >>> + */ >>> +struct futex_waitv { >>> + __u64 val; >>> + __u64 uaddr; >>> + __u32 flags; >>> + __u32 __reserved; >>> +}; >> >> why force uaddr to be __u64, even for 32-bit? uaddr could be a (void*) for >> all we care, no? Also, by adding a reserved field, you are wasting 32 >> bits even on 32-bit architectures. >> > > We do that to make the structure layout compatible with both entry > points, remove the need for special cast and duplicated code, as > suggested by Thomas and Arnd: > > https://lore.kernel.org/lkml/87v94310gm.ffs@tglx/ > > https://lore.kernel.org/lkml/CAK8P3a0MO1qJLRkCH8KrZ3+=L66KOsMRmcbrUvYdMoKykdKoyQ@mail.gmail.com/ I find this weird. I'm not even juts talking about compat, but even on native 32-bit. But also, 32 applications on 64, which is a big use case for games. The structure is mandating a 64 bit uaddr field and has an unnecessary pad. You are wasting 20% of the space, which is gonna be elements of a vector coming from user space. Worst case, you are doing copy_from_user of an extra 1k bytes in the critical path of futex_waitv for no good reason. Also, if I understand correctly, Arnd suggestion, at least, was to have two parser functions and a single syscall entry point, that would do the translation: if (in_compat_syscall()) futex_parse_waitv_compat(futexv, waiters, nr_futexes); else futex_parse_waitv(futexv, waiters, nr_futexes); -- Gabriel Krisman Bertazi