Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-1178-geeaf0069a7-fm-20231114.001-geeaf0069
MIME-Version: 1.0
Message-Id: <67fab0f1-e326-4ad8-9def-4d2bd5489b33@app.fastmail.com>
In-Reply-To: <20231207002759.51418-8-gregory.price@memverge.com>
References: <20231207002759.51418-1-gregory.price@memverge.com>
 <20231207002759.51418-8-gregory.price@memverge.com>
Date:   Thu, 07 Dec 2023 08:13:22 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Gregory Price" <gourry.memverge@gmail.com>, linux-mm@kvack.org,
        jgroves@micron.com, ravis.opensrc@micron.com,
        sthanneeru@micron.com, emirakhur@micron.com, Hasan.Maruf@amd.com
Cc:     linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-api@vger.kernel.org, Linux-Arch <linux-arch@vger.kernel.org>,
        linux-kernel@vger.kernel.org,
        "Andrew Morton" <akpm@linux-foundation.org>,
        "Thomas Gleixner" <tglx@linutronix.de>,
        "Andy Lutomirski" <luto@kernel.org>,
        "Ingo Molnar" <mingo@redhat.com>, "Borislav Petkov" <bp@alien8.de>,
        "Dave Hansen" <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        "Michal Hocko" <mhocko@kernel.org>, "Tejun Heo" <tj@kernel.org>,
        ying.huang@intel.com, "Gregory Price" <gregory.price@memverge.com>,
        "Jonathan Corbet" <corbet@lwn.net>, rakie.kim@sk.com,
        hyeongtak.ji@sk.com, honggyu.kim@sk.com, vtavarespetr@micron.com,
        "Peter Zijlstra" <peterz@infradead.org>,
        "Frank van der Linden" <fvdl@google.com>
Subject: Re: [RFC PATCH 07/11] mm/mempolicy: add userland mempolicy arg structure
Content-Type: text/plain
Precedence: bulk

On Thu, Dec 7, 2023, at 01:27, Gregory Price wrote:
> This patch adds the new user-api argument structure intended for
> set_mempolicy2 and mbind2.
>
> struct mpol_args {
>   /* Basic mempolicy settings */
>   unsigned short mode;
>   unsigned short mode_flags;
>   unsigned long *pol_nodes;
>   unsigned long pol_maxnodes;
>
>   /* get_mempolicy2: policy information (e.g. next interleave node) */
>   int policy_node;
>
>   /* get_mempolicy2: memory range policy */
>   unsigned long addr;
>   int addr_node;
>
>   /* all operations: policy home node */
>   unsigned long home_node;
>
>   /* mbind2: address ranges to apply the policy */
>   const struct iovec __user *vec;
>   size_t vlen;
> };

This is not a great structure layout for a system call ABI,
mostly because it requires adding a compat syscall handler
to be usable from 32-bit tasks. It would be nice if this
could be rewritten in a way that uses only fixed-length
members (__u16, __u32, __aligned_u64), though that does
require the use of u64_to_user_ptr() to replace the pointers
and the reverse in userspace.

Aside from this, you should avoid holes in the data structure.
On 64-bit architectures, the layout above has holes after
policy_node and after addr_node.

      Arnd