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 B32A9CF9C6C for ; Wed, 25 Sep 2024 14:03:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2F01D6B00AD; Wed, 25 Sep 2024 10:03:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 29F796B00AE; Wed, 25 Sep 2024 10:03:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 167AB6B00AF; Wed, 25 Sep 2024 10:03:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id ED5606B00AD for ; Wed, 25 Sep 2024 10:03:14 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 2DF271C6B70 for ; Wed, 25 Sep 2024 14:03:14 +0000 (UTC) X-FDA: 82603427508.18.28EEC1F Received: from out-175.mta0.migadu.com (out-175.mta0.migadu.com [91.218.175.175]) by imf11.hostedemail.com (Postfix) with ESMTP id 161ED40005 for ; Wed, 25 Sep 2024 14:03:09 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=XiinWLMV; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf11.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727272976; a=rsa-sha256; cv=none; b=uVYqtzh2VAMd977H2ds996fS2pTSnUU+8xM21s9aw4tuBS1LE1LRdnpFIVEvYotZRd05N+ j+JV5PEjLPnVaG1LWdT5820duN3XLabxRQQrZVOTgMywEPw6tC9zdS/8/Nasdbhgf6DaT1 uavK1JrokL7odPiuEXDjfr9H+FJgNFA= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=XiinWLMV; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf11.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727272976; 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=5t463NG6eQ9jLF1YTXL/0TjCocLmn8Mda+WfoUOloPc=; b=MseFBX/dYik4EAK+/Q1uy13DfblGRfaFSj9x5/paiOq4nyHeB8eYhaSfa5HLO5VkeZ8xsZ MksbrKrm7GYbVder/1mmSKGq4KFETg/cMzwu40WWLQBx9rgmWJcC+kvQvQZcEhMLhtMVFc RcdsJ66sEUP/b5bOIVY4eJIiKlm4CW8= Date: Wed, 25 Sep 2024 07:02:59 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1727272987; 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=5t463NG6eQ9jLF1YTXL/0TjCocLmn8Mda+WfoUOloPc=; b=XiinWLMVgLAFXBHkWWg+stYNqT4HftwpWwql5zrUkGf56VIJnBycuq0wrsWFr53xHFZd2Q pfJNKS/UZ8nX2VTTiyi01vbdQ0fKa9LlV3Jg30rG4uGmRQDh6enLUC+J6y/Zt/6UhKNdWU yVfeEOwFmSfpi6DyFPWg8LXvIYzV6sM= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Lorenzo Stoakes Cc: Pedro Falcato , Andrew Morton , Vlastimil Babka , "Liam R . Howlett" , Suren Baghdasaryan , Arnd Bergmann , linux-api@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Minchan Kim , Richard Henderson , Ivan Kokshaysky , Matt Turner , linux-alpha@vger.kernel.org, Thomas Bogendoerfer , linux-mips@vger.kernel.org, "James E . J . Bottomley" , Helge Deller , linux-parisc@vger.kernel.org, Chris Zankel , Max Filippov , christian@brauner.io Subject: Re: [PATCH v2 1/2] mm/madvise: introduce PR_MADV_SELF flag to process_madvise() Message-ID: References: <1ecf2692b3bcdd693ad61d510ce0437abb43a1bd.1727176176.git.lorenzo.stoakes@oracle.com> <4740dfc7-71da-4eb4-b071-35116288571f@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4740dfc7-71da-4eb4-b071-35116288571f@lucifer.local> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Stat-Signature: wg77yxxits4pg61x1oiwyh4hbwk5mkd3 X-Rspamd-Queue-Id: 161ED40005 X-Rspamd-Server: rspam02 X-HE-Tag: 1727272989-619694 X-HE-Meta: U2FsdGVkX18u0pkX2/St9kZiXI1ltjJPQBrVTk0WXvFI/YUQl5RZiIBil+VPjMQb0/onpd20eQk4CMNxSaelyHFjLDtGb7x2O95c3la90G2+Hmq6CMvaijPbQ5v+GW+i5eOqFKfXj/6cgV8QpSxwbqVDsJ8uulWxGcjijN19TZxmSe0K+SWCRnXSwoC0N3qmancG1e9yTOILDEWQg+kRAw7ILNiAXEd/9tJVTNHQAHPCW4vdYNItjpwQZcBjR1X5hf7cE5rqSs2wxNCCM8Z20RqO3mE2nIwoRzu9GOXZMt5KrtuttNrd+GE+OlCz1FYExkv0uvYwHQHv5C7MGFB6Np0/Dq0AxE+yN5YvMLRyhX7ELUDXEblgyBER7CHGS5mpBZxN5y3S+vhiqiE7R0SFXyPSysaT2h5Bf3DEZhNT4cUrrOyVD6/7sQMuHUGds8/Ur6rGxt3JNaEWom/wp8VfEMxLQvMvE/OMlnojuzmaygAyhJhXlOz9nNkPjmva5XuCt+ehxQtSj6cNaEHjKALuMboPrHzrLEtNGp/KM25Uv9V2S+wl9u52krfA/KV0QEybXvFHb2G+kqTHGhCD2Ki2J1QAwcqcaLP8o4I1n6QUf1JZajiPebmi5jbwq/Cc2k1xyYmZT1SLVGuKGjtVH4AadMgyzgpvGvQf+TC5C6Yod8O+otPVHVoOcm8PFCzX3Y5KznSNRCBX3NuqpWowF2SXfgRUMot6KYkAYd9WxiSyhwAph4fH/rQyRfiaOdhUNcLSbGGy+EKpIACXsYocalZQ0DA07iYmz/JtXGuyAC07jXj16hnGfEyJ2OC8QrQTp2Df57Cfcdh4cT/LR/3hXcRDwnGuv8jqba9ccaDWQ/zYpWkXK9h1dOwftUGqWXnqJG7sY6z89YsVMxNQvDMrhVv4pitslmz/5y1PIHFjZe/plw6ef4pV2mY0dJIlcJVYbLCx2ckQ1mcV0ketvpsGrvH 60TdDMm/ XT+ePncRlL8tJClCcntMmDpdcJJwosSFxQx+3hDdXF0nlZcwohmMjwRwz7K5x0N3ARXPCY1jfxGRZPll+gZcX/HmHaaA/BfPArMoqYGGfVosXCjgF3W2JbXRs4IV5rhio2hJIwwCFrNBio0ILIqjm2T/TLuHCI/vq+nsJ19QhxELDoUZT6ST4eOhYzfxgF9Iw2DUYAjOaMEFukpbNLV1/46AVHs4K3v8X8gsQgaJWqW7n6CP852kKrTVwhQVjQtZGKkeey1vKZDbZ6uw= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Cced Christian On Tue, Sep 24, 2024 at 02:12:49PM GMT, Lorenzo Stoakes wrote: > On Tue, Sep 24, 2024 at 01:51:11PM GMT, Pedro Falcato wrote: > > On Tue, Sep 24, 2024 at 12:16:27PM GMT, Lorenzo Stoakes wrote: > > > process_madvise() was conceived as a useful means for performing a vector > > > of madvise() operations on a remote process's address space. > > > > > > However it's useful to be able to do so on the current process also. It is > > > currently rather clunky to do this (requiring a pidfd to be opened for the > > > current process) and introduces unnecessary overhead in incrementing > > > reference counts for the task and mm. > > > > > > Avoid all of this by providing a PR_MADV_SELF flag, which causes > > > process_madvise() to simply ignore the pidfd parameter and instead apply > > > the operation to the current process. > > > > > > > How about simply defining a pseudo-fd PIDFD_SELF in the negative int space? > > There's precedent for it in the fs space (AT_FDCWD). I think it's more ergonomic > > and if you take out the errno space we have around 2^31 - 4096 available sentinel > > values. > > > > e.g: > > > > /* AT_FDCWD = -10, -1 is dangerous, pick a different value */ > > #define PIDFD_SELF -11 > > > > int pidfd = target_pid == getpid() ? PIDFD_SELF : pidfd_open(...); > > process_madvise(pidfd, ...); > > > > > > What do you think? > > I like the way you're thinking, but I don't think this is something we can > do in the context of this series. > > I mean, I totally accept using a flag here and ignoring the pidfd field is > _ugly_, no question. But I'm trying to find the smallest change that > achieves what we want. I don't think "smallest change" should be the target. We are changing user API and we should aim to make it as robust as possible against possible misuse or making uninteded assumptions. The proposed implementation opened the door for the applications to provide dummy pidfd if PR_MADV_SELF is used. You definitely need to restrict it to some known value like -1 used by mmap() syscall. > > To add such a sentinel would be a change to the pidfd mechanism as a whole, > and we'd be left in the awkward situation that no other user of the pidfd > mechanism would be implementing this, but we'd have to expose this as a > general sentinel value for all pidfd users. There might be future users which can take advantage of this. I can even imagine pidfd_send_signal() can use PIDFD_SELF as well. > > One nice thing with doing this as a flag is that, later, if somebody is > willing to do the larger task of having a special sentinel pidfd value to > mean 'the current process', we could use this in process_madvise() and > deprecate this flag :) > Once something is added to an API, particularly syscalls, the removal is almost impossible. Anyways, I don't have very strong opinion one way or other but whatever we decide, let's make it robust.