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 79060CE8349 for ; Mon, 30 Sep 2024 14:21:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E3B506B0294; Mon, 30 Sep 2024 10:21:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DEBFE6B0325; Mon, 30 Sep 2024 10:21:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CB5136B0326; Mon, 30 Sep 2024 10:21:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id AF5786B0294 for ; Mon, 30 Sep 2024 10:21:53 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 554968053E for ; Mon, 30 Sep 2024 14:21:53 +0000 (UTC) X-FDA: 82621618506.04.A455186 Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) by imf25.hostedemail.com (Postfix) with ESMTP id 4AA2CA0002 for ; Mon, 30 Sep 2024 14:21:50 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=cyphar.com header.s=MBO0001 header.b=w9ynys3f; spf=pass (imf25.hostedemail.com: domain of cyphar@cyphar.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=cyphar@cyphar.com; dmarc=pass (policy=reject) header.from=cyphar.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727705984; 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=AbDlpohMGyKbU24tJpU7s9PNV5HIsnEIUJ337YJEe0E=; b=hTR2Uyi6A+aTaNh4g+FpM6KivLTg6sUs/aFtxoTMn5lyI51HcGU5dTjYBOeKpVLKz5PePx yhpYzh0/3PDuU0pAprW5ueZuqB4asXYv3poZUpKoZzojzvTIobmoRWFYPc3eOD6V6FJiS2 0bDOxeM3+EvfKRXOfpMRSRPtbC+tISc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727705984; a=rsa-sha256; cv=none; b=yFvhVCYsMrphWsAzBO769qrffkHkw39lUiZKFcJOBFLET74EU3Ig9k2f6iDBppJCVptRB1 4Vms32fFN4Pj8l4N/xvTHSJdjj31ptOhRCEJCLP85zo/rR5UCLguIvl+cU7cSWoJBayzdJ Rw6IuTLfIRfMw11tW3m/Md10glnqxGM= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=cyphar.com header.s=MBO0001 header.b=w9ynys3f; spf=pass (imf25.hostedemail.com: domain of cyphar@cyphar.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=cyphar@cyphar.com; dmarc=pass (policy=reject) header.from=cyphar.com Received: from smtp1.mailbox.org (smtp1.mailbox.org [10.196.197.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4XHNYN2m3Jz9sPn; Mon, 30 Sep 2024 16:21:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cyphar.com; s=MBO0001; t=1727706104; 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=AbDlpohMGyKbU24tJpU7s9PNV5HIsnEIUJ337YJEe0E=; b=w9ynys3f5TGtKkd8kaUSuPTTOef++CqXfd5FlZGlDnB6B/xA6kPvZEcSaIl8ntCz/49z6O wZdzkCWh8n7l156xTCAMsWG2dBluZVnuG1OCVy25ckbImIvH94K9GDJCXWUAA/0/XNDUcV JLwKDNq6CetEYGIUNqER0yZoDtdsJ1YIrcy08Hx0MxzfHGez3VNox2cZLqrc6FuyBICJdX JSA8vqb0v6pHlHxFxORqoiCUcfXVkPE6bbkMUPNkJ3D9u8m1P4qCa0Y3yxs1eCpKpciO3o zzTF/gqG2PTE3ASWFngVEo/E0dk5lBcFXzvnLi7sJO8H73LcyPTAO3ob9sbYrQ== Date: Mon, 30 Sep 2024 16:21:23 +0200 From: Aleksa Sarai To: Lorenzo Stoakes Cc: Christian Brauner , Florian Weimer , Christian Brauner , Shuah Khan , "Liam R . Howlett" , Suren Baghdasaryan , Vlastimil Babka , pedro.falcato@gmail.com, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 0/3] introduce PIDFD_SELF Message-ID: <20240930.141721-salted.birth.growing.forges-5Z29YNO700C@cyphar.com> References: <87ttdxl9ch.fsf@oldenburg.str.redhat.com> <42df57ac-d89c-4111-a04d-290dd2197573@lucifer.local> <20240930-verbiegen-zinspolitik-cafb730c3c84@brauner> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="6a2j2i22ruyltnqf" Content-Disposition: inline In-Reply-To: X-Stat-Signature: adspe8p46cgrwc36fbcecfgcict7mxoq X-Rspamd-Queue-Id: 4AA2CA0002 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1727706110-41448 X-HE-Meta: U2FsdGVkX19Td1IGJvfIMsq2T7pbkbdIOJV7AxB3cNeWH3t+daBJjs9CVjIkEzktFA7MO2u3oaHRYC0V90eT94wc/dwYgCyfCLNBAHE/QxsJ0LpJqiCVgAP0MEasaFwU/d5Gki1e5rvFOFG35zoTKbFKQLro3i3nBAMHPnxmzwShj8xcuLC/Kslblxh6Xu/ijKi5NY2Ydd1S+yvj1gISewVMPGxv18uhqFM39Ep3mVkbNi50mMKYPyxhvoZsmhhRLC/RmhiScYyBtzdYjd7O0kAduSXTTInhCelgLYcikkav1v8KGH7ji3oRkgD0huZOdEFYoXZ7hePtoCLj3sxRoGz4b84s+xsDu2X73Q6SOvhS4VwwRG5PX67qJzO6yBKutD6chDVWR15jcAebG5fgLQCzfXBzX8XOmcwdI2JHjhG6dhCipUtNMeU9i+3jBD69CWi+jDeEqVuQ3lvl/NTHc0sDgeV/gIRfmL+fymsnhbtJDN4AtNX0lXzatqKLTpxK7AANSEZebgIonlxmE5Btkk5STtGFDKsK9gAwQSBCCQ8IYcm62RIpyHW0Cn79Cj9BBOIAIjPxEHPgafdtQYGSaNlB0YsSepTPgYunkJPxocguZCSyVif/XTqYUwFL3YGvz57wxpOnvP67f9D5nO/M9y/PFSmqp5HRWdnwemC28ZguoS8b1LBbgfzjIdaLFc7HAsEoVocy7w69r5Lct6It1PtZXGYspNsCpqVl9+k3A8nzleo/CDIYnDhywJ8j8YK0zMAsyVWK1Y9lm00WKAVP6BwAO7Dz/SzJ6nv9E6a2cyPvRAmIOQGfhDvYWi0WurD3I1iXSYRR3tQZVZn0PJVJZ50a+tnTOFKkkyVnAFiwQalxBFSdWjaGDjzIfeAM1Bt6MRUqNnNFL9R883rSwdA+gJuj3Ov3dZArWojXuxlJWwiKdF4bO4eFeZUw8amRpO/qxl77VH+SPyMylDnR0p3 APlpMbbB bPMN51z0q/ls+tep4x+ZJpYT5303b+GXLLRDWdDetk7wcu/0X9A1GTemXuX6Q4BxOX+wGEtcD3gybC1vNN0LtJvY+4r5uDP7vKiF9kDlF20EH/3MCt/2V2KKoX0iXysMQMW7FmXWm0e9xm9fEZlaFJ+extFSX9RrAVEwWMtZBO1jAtZn5C+Ej0eKbgs/LXijNMFgq6d6GXzeE4NZqyDvcDwMGt5MvYDlI1tSduVgtlaQspUdaTxGdlVwpiGfbQdofS9rfmTTb94dTppWwj5HAgFhtOINXPONBRbuI/vE52CS20Ql7A7c3mlAcQwtVRkzJByMuy+Kz2HEa9VjTZIk6tDNhZuihF+QRLSiqreJAbiJwoln2NaQg3p/8BV0WzgmDyFHdbIVXeFIa8wE9RZwbxp++KJxaSc4yd8RY9UbDVBEAHs5wZ9ZNtSsME2tHvdfabX6SEDy6JIbk2X3kWX/qtsuWCdWq/l23qlSfHjBa1cDn7CW/D7cH5sfx67zMc8rqZjH7kXGaJTo2BWg= 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: List-Subscribe: List-Unsubscribe: --6a2j2i22ruyltnqf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2024-09-30, Lorenzo Stoakes wrote: > On Mon, Sep 30, 2024 at 02:34:33PM GMT, Christian Brauner wrote: > > On Mon, Sep 30, 2024 at 11:39:49AM GMT, Lorenzo Stoakes wrote: > > > On Mon, Sep 30, 2024 at 12:33:18PM GMT, Florian Weimer wrote: > > > > * Lorenzo Stoakes: > > > > > > > > > If you wish to utilise a pidfd interface to refer to the current = process > > > > > (from the point of view of userland - from the kernel point of vi= ew - the > > > > > thread group leader), it is rather cumbersome, requiring somethin= g like: > > > > > > > > > > int pidfd =3D pidfd_open(getpid(), 0); > > > > > > > > > > ... > > > > > > > > > > close(pidfd); > > > > > > > > > > Or the equivalent call opening /proc/self. It is more convenient = to use a > > > > > sentinel value to indicate to an interface that accepts a pidfd t= hat we > > > > > simply wish to refer to the current process. > > > > > > > > The descriptor will refer to the current thread, not process, right? > > > > > > No it refers to the current process (i.e. thread group leader from ke= rnel > > > perspective). Unless you specify PIDFD_THREAD, this is the same if yo= u did the above. > > > > > > > > > > > The distinction matters for pidfd_getfd if a process contains multi= ple > > > > threads with different file descriptor tables, and probably for > > > > pidfd_send_signal as well. > > > > > > You mean if you did a strange set of flags to clone()? Otherwise thes= e are > > > shared right? > > > > > > Again, we are explicitly looking at process not thread from userland > > > perspective. A PIDFD_SELF_THREAD might be possible, but this series d= oesn't try > > > to implement that. > > > > Florian raises a good point. Currently we have: > > > > (1) int pidfd_tgid =3D pidfd_open(getpid(), 0); > > (2) int pidfd_thread =3D pidfd_open(getpid(), PIDFD_THREAD); > > > > and this instructs: > > > > pidfd_send_signal() > > pidfd_getfd() > > > > to do different things. For pidfd_send_signal() it's whether the > > operation has thread-group scope or thread-scope for pidfd_send_signal() > > and for pidfd_getfd() it determines the fdtable to use. > > > > The thing is that if you pass: > > > > pidfd_getfd(PDIFD_SELF) > > > > and you have: > > > > TGID > > > > T1 { > > clone(CLONE_THREAD) > > unshare(CLONE_FILES) > > } > > > > T2 { > > clone(CLONE_THREAD) > > unshare(CLONE_FILES) > > } > > > > You have 3 threads in the same thread-group that all have distinct file > > descriptor tables from each other. > > > > So if T1 did: > > > > pidfd_getfd(PIDFD_SELF, ...) > > > > and we mirror the PIDTYPE_TGID behavior then T1 will very likely expect > > to get the fd from its file descriptor table. IOW, its reasonable to > > expect that T1 is interested in their very own resource, not someone > > else's even if it is the thread-group leader. > > > > But what T1 will get in reality is an fd from TGID's file descriptor > > table (and similar for T2). > > > > Iirc, yes that confusion exists already with /proc/self. But the > > question is whether we should add the same confusion to the pidfd api or > > whether we make PIDFD_SELF actually mean PIDTYPE_PID aka the actual > > calling thread. > > > > My thinking is that if you have the reasonable suspicion that you're > > multi-threaded and that you're interested in the thread-group resource > > then you should be using: > > > > int pidfd =3D pidfd_open(getpid(), 0) > > > > and hand that thread-group leader pidfd around since you're interested > > in another thread. But if you're really just interested in your own > > resource then pidfd_open(getpid(), 0) makes no sense and you would want > > PIDFD_SELF. > > > > Thoughts? >=20 > I mean from my perspective, my aim is to get current->mm for > process_madvise() so both work for me :) however you both raise a very go= od > point here (sorry Florian, perhaps I was a little too dismissive as to yo= ur > point, you're absolutely right). >=20 > My intent was for PIDFD_SELF to simply mirror the pidfd_open(getpid(), 0) > behaviour, but you and Florian make a strong case that you'd _probably_ > find this very confusing had you unshared in this fashion. >=20 > I mean in general this confusion already exists, and is for what > PIDFD_THREAD was created, but I suspect ideally if you could go back you > might actually do this by default Christian + let the TGL behaviour be the > optional thing? >=20 > For most users this will not be an issue, but for those they'd get the sa= me > result whichever they used, but yes actually I think you're both right - > PIDFD_SELF should in effect imply PIDFD_THREAD. Funnily enough we ran into issues with this when running Go code in runc that did precisely this -- /proc/self gave you the wrong fd table in very specific circumstances that were annoying to debug. For languages with green-threading you can't turn off (like Go) these kinds of issues pop up surprisingly often. > We can adjust the pidfd_send_signal() call to infer the correct scope > (actually nicely we can do that without any change there, by having > __pidfd_get_pid() set f_flags accordingly). >=20 > So TL;DR: I agree, I will respin with PIDFD_SELF referring to the thread. >=20 > My question in return here then is - should we introduce PIDFD_SELF_PROCE= SS > also (do advise if you feel this naming isn't quite right) - to provide > thread group leader behaviour? Sorry to bike-shed, but to match /proc/self and /proc/thread-self, maybe they should be called PIDFD_SELF (for tgid) and PIDFD_THREAD_SELF (for current's tid)? In principle I guess users might use PIDFD_SELF by accident but if we mirror the naming with /proc/{,thread-}self that might not be that big of an issue? Just a thought. >=20 > Thanks! >=20 --=20 Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH --6a2j2i22ruyltnqf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQS2TklVsp+j1GPyqQYol/rSt+lEbwUCZvqz4wAKCRAol/rSt+lE b7GGAQChOyMNzOC/RM56OVrgBcSbrRYTofZfidupfTSRne9UVAEArUjap0YYofOn VdJnnofMreht73y08E5gdexZZ9eZmg4= =CCUx -----END PGP SIGNATURE----- --6a2j2i22ruyltnqf--