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=-5.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 A0D55C28CBC for ; Sat, 9 May 2020 12:48:25 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 449E320820 for ; Sat, 9 May 2020 12:48:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 449E320820 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ubuntu.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8D3D1900005; Sat, 9 May 2020 08:48:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 883438E0003; Sat, 9 May 2020 08:48:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7998A900005; Sat, 9 May 2020 08:48:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0116.hostedemail.com [216.40.44.116]) by kanga.kvack.org (Postfix) with ESMTP id 5DF468E0003 for ; Sat, 9 May 2020 08:48:24 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 1C7A33CE9 for ; Sat, 9 May 2020 12:48:24 +0000 (UTC) X-FDA: 76797158928.03.end28_494db9225021a X-HE-Tag: end28_494db9225021a X-Filterd-Recvd-Size: 5233 Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by imf40.hostedemail.com (Postfix) with ESMTP for ; Sat, 9 May 2020 12:48:23 +0000 (UTC) Received: from ip5f5af183.dynamic.kabel-deutschland.de ([95.90.241.131] helo=wittgenstein) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jXOtv-0003Jb-3t; Sat, 09 May 2020 12:48:19 +0000 Date: Sat, 9 May 2020 14:48:17 +0200 From: Christian Brauner To: Andrew Morton Cc: Minchan Kim , Vlastimil Babka , LKML , linux-mm , linux-api@vger.kernel.org, oleksandr@redhat.com, Suren Baghdasaryan , Tim Murray , Daniel Colascione , Sandeep Patil , Sonny Rao , Brian Geffon , Michal Hocko , Johannes Weiner , Shakeel Butt , John Dias , Joel Fernandes , Jann Horn , alexander.h.duyck@linux.intel.com, sj38.park@gmail.com, Christian Brauner , Kirill Tkhai Subject: Re: [PATCH v7 5/7] mm: support both pid and pidfd for process_madvise Message-ID: <20200509124817.xmrvsrq3mla6b76k@wittgenstein> References: <20200302193630.68771-1-minchan@kernel.org> <20200302193630.68771-6-minchan@kernel.org> <14089609-5fb1-b082-716f-c2e129d27c48@suse.cz> <20200311004251.GB87930@google.com> <20200508183653.GB125527@google.com> <20200508160415.65ff359a9e312c613336587b@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200508160415.65ff359a9e312c613336587b@linux-foundation.org> 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, May 08, 2020 at 04:04:15PM -0700, Andrew Morton wrote: > On Fri, 8 May 2020 11:36:53 -0700 Minchan Kim wrote: > > > > > ... > > > > Per Vlastimil's request, I changed "which and advise" with "idtype and > > advice" in function prototype of description. > > Could you replace the part in the description? Code is never changed. > > > > Done, but... > > > > > ... > > > > There is a demand[1] to support pid as well pidfd for process_madvise to > > reduce unnecessary syscall to get pidfd if the user has control of the > > target process(ie, they could guarantee the process is not gone or pid is > > not reused). > > > > This patch aims for supporting both options like waitid(2). So, the > > syscall is currently, > > > > int process_madvise(idtype_t idtype, id_t id, void *addr, > > size_t length, int advice, unsigned long flags); > > > > @which is actually idtype_t for userspace libray and currently, it > > supports P_PID and P_PIDFD. > > What does "@which is actually idtype_t for userspace libray" mean? Can > you clarify and expand? If I may clarify, the only case where we've supported both pidfd and pid in the same system call is waitid() to avoid adding a dedicated system call for waiting and because waitid() already had this (imho insane) argument type switching. The idtype_t thing comes from waitid() and is located int sys/wait.h and is defined as "The type idtype_t is defined as an enumeration type whose possible values include at least the following: P_ALL P_PID P_PGID " int waitid(idtype_t idtype, id_t id, siginfo_t *infop, int options); If idtype is P_PID, waitid() shall wait for the child with a process ID equal to (pid_t)id. If idtype is P_PGID, waitid() shall wait for any child with a process group ID equal to (pid_t)id. If idtype is P_ALL, waitid() shall wait for any children and id is ignored. I'm personally not a fan of this idtype_t thing and think this should just have been > > int pidfd_madvise(int pidfd, void *addr, > > size_t length, int advice, unsigned long flags); and call it a day. Also, if I may ask, why is the flag argument "unsigned long"? That's pretty unorthodox. The expectation is that flag arguments are not word-size dependent and should usually use "unsigned int". All new system calls follow this pattern too. The current syscall layout will mean that on 64 bit systems you have 64 flag bits and on 32 bit you have 32 flag bits, I think. That has just recently led to some problems with the clone() syscall (fixed in [1] which I'm sending Monday) which has the same weird word-size-dependent flag argument layout. If a system does sign-extension and a userspace api or glibc uses e.g. an int for the flag argument in the system call wrapper - which is fairly common - you can get sign extended and then you end up with garbage in the upper 32 bits of your system call. > > Also, does this userspace library exist? If so, where is it? [1]: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/commit/?h=fixes&id=3f2c788a13143620c5471ac96ac4f033fc9ac3f3 Christian