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 3F23FC77B7A for ; Tue, 13 Jun 2023 08:48:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BC7208E000B; Tue, 13 Jun 2023 04:48:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B77888E0002; Tue, 13 Jun 2023 04:48:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F3998E000B; Tue, 13 Jun 2023 04:48:42 -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 8A9198E0002 for ; Tue, 13 Jun 2023 04:48:42 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 5AF67C0426 for ; Tue, 13 Jun 2023 08:48:42 +0000 (UTC) X-FDA: 80897098884.07.D6A37D2 Received: from mail-vs1-f54.google.com (mail-vs1-f54.google.com [209.85.217.54]) by imf19.hostedemail.com (Postfix) with ESMTP id 7E69F1A0004 for ; Tue, 13 Jun 2023 08:48:39 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=ErwzaYxz; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf19.hostedemail.com: domain of sumit.garg@linaro.org designates 209.85.217.54 as permitted sender) smtp.mailfrom=sumit.garg@linaro.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686646119; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=+moT08wPsJXStgU7vMdHROE2qSZd8ki23/kaFbG+8IY=; b=ClxXFMKTZzXCUDUWJhHcmhjJXGnRRhfCwizSw03pTzB7k04PKTyhV32lhJA0VJiX8o31gz Z8yYAlcdxFXi3S2f1qU9pRzpjKuxTaLw1CXKJHFo/BqNmFSojNDqRvWyOI5+YN4BzJxF1M Q8WVcIeYTI+/BRaesHJ2j7Qe0nrLsAI= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=ErwzaYxz; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf19.hostedemail.com: domain of sumit.garg@linaro.org designates 209.85.217.54 as permitted sender) smtp.mailfrom=sumit.garg@linaro.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686646119; a=rsa-sha256; cv=none; b=N9lfBRNZvY8RKV7Q5Nu6BQlx8PIewXZBztBz1MPtDDFS5DE73e2l4Jgwgdqhgru9aTSJhz mIL43U3rLX1nWO5aMi0TpxtCry1wlmwF07DMPWRpeXsNDbkCrjyL2Ux3OD0F3se8rukF1I mwyWZbcQp7ZEWqUeAw/kPggTlLJ8MX0= Received: by mail-vs1-f54.google.com with SMTP id ada2fe7eead31-43dc17feb22so298487137.3 for ; Tue, 13 Jun 2023 01:48:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1686646118; x=1689238118; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=+moT08wPsJXStgU7vMdHROE2qSZd8ki23/kaFbG+8IY=; b=ErwzaYxz4Bp9CQ96lMEydtXpgOFdCh7wlLrgOvSNsLImmkePJEHijWTLhcRn+hAmis dlrSa2gO2vtYGdk46wdYv8fx4LHu7cR2ieuAlObzziFtM+r/p1V4KLRkdM6YEixoSxov ee41NZPG05/ZbJ6YuBgfNWLpMvL4reNL08FT2ccHk4fR4DdIDBLyqxsvG079slfWh/YU 4Vowa3gYhguYwGMWJoTeAYBPSrbdoQ7vtC7F76+ptZDj1QWdENWImyk071utAYcxSDqP vYK48cx/c3fYEPawKr1JoLUzRm2HxiwotvJ440dUEEy2LQbjdrppT9Ur0jagj4QFrNW6 FIXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686646118; x=1689238118; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+moT08wPsJXStgU7vMdHROE2qSZd8ki23/kaFbG+8IY=; b=d2oNWZaJ1N60UPZCklC3WT8cHeKHAFgb+nw5GiB1XRzoIU0/yMd7LrFq41zZLzmA2p BSmPIdxwXCqDAJRKqaI+NiIRnNJqV1xVmnJDXnAxdpD1rcIAPTM5B9XH/tw2F+hgljPs DkJdKOT/z5gTTCgWqkgYGIEowpTWSwg9gPr9SzSMIb2C2XjDtvbNHPqTB5wYCkl/J7xA VYB8bTiIBbriX7ydKAcFqhGFN4B7tF5LZCEx5zR8tPmNE5MJyQ98UloEPrJLDLvl0jnM 8lBLh++wQyEuW02QImEJuLwKCPhA9xBKXupbCsDlzAiP0/rwfilsJke8I9Hg93ak8edV wKFw== X-Gm-Message-State: AC+VfDxwMjTBO5Pz5q643Ppv6+CI1H+OCY1ljH40R6hk6p2q/EHqbN5m rW7/Usohqe3BL0vIjpAB/e2zHDCzNC6854a1BpwCEw== X-Google-Smtp-Source: ACHHUZ5z48J7c1Hn2zzF7MPqVJPcmVwjrmcj6TW3W2znJMoNGF2OjYpOPFKoZpQX7oA/IWduT0SdafIEL6t+JjkCWXM= X-Received: by 2002:a67:fa46:0:b0:43c:886a:f0c9 with SMTP id j6-20020a67fa46000000b0043c886af0c9mr5439707vsq.5.1686646118541; Tue, 13 Jun 2023 01:48:38 -0700 (PDT) MIME-Version: 1.0 References: <8b898768-a3c0-198d-a70c-9aa96b7f4a29@redhat.com> <1ced7f32-553e-2a5b-eec9-f794d7983d56@nvidia.com> <106e7886-ef02-4979-b96d-66d33cfa119c@lucifer.local> In-Reply-To: From: Sumit Garg Date: Tue, 13 Jun 2023 14:18:27 +0530 Message-ID: Subject: Re: FOLL_LONGTERM vs FOLL_EPHEMERAL Re: [PATCH] tee: add FOLL_LONGTERM for CMA case when alloc shm To: =?UTF-8?B?WGlhb21pbmcgRGluZyAo5LiB5pmT5piOKQ==?= Cc: "lstoakes@gmail.com" , "jhubbard@nvidia.com" , "linux-kernel@vger.kernel.org" , "linux-mediatek@lists.infradead.org" , "hch@infradead.org" , =?UTF-8?B?RmVpIFh1ICjlvpDpo54p?= , "linux-mm@kvack.org" , "david@redhat.com" , "srv_heupstream@mediatek.com" , "jens.wiklander@linaro.org" , "op-tee@lists.trustedfirmware.org" , "linux-arm-kernel@lists.infradead.org" , "matthias.bgg@gmail.com" , "angelogioacchino.delregno@collabora.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 7E69F1A0004 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: uzyj3ik1sibyqqnmcyoi4k4hb91wn8qm X-HE-Tag: 1686646119-290254 X-HE-Meta: U2FsdGVkX1+N6BJAz/BYU8uzRqgdpwqRO6Bkqlo1wGwgukDCGzkSMKHwgQIsbN5Zh58xmVok6ANp6sdIAY+cF5VnXGk90ojdNSW1ti5/CnHu+MnIhdH+72d/PSMup1IJAGqQ8QEydkVnJ5TES8wxIE4gobM7iehRYe+UWprgJEfOW81v77v2+DXcRAOggxuWWSDq5ZLtsJT4V0Zi7agwj+QloLh1FNbrXLs7uT31n+MVyHe5boyMjKvcVM0LhFAyoAxxsPyM3ReA+uTdQP7WkoLTsga3sJGpJxzTUEYNaaN5eGSFFyaXnHknQxkTRRz2dKwnpWsE+9UoxI1REEtG5fwXmabrvyt1HL6+SN4kRKU0Mdanjrd3SQeRW3dJVdo21x7BjveEwQk47/PYQlc7vlkeTNmh+3nhcTZZaeb22RE5xevsxUhMSsF8xPXnvK0qBksVyOgg43p8It2vb/Oa4cQsjDzxqannYLQRGhZRsDJ1esMUrinFEBWzQZ/9PLcA7S6/R4GbjVhOFbTtNGf8X4OaT+5ltaeVsECssdMOgXU3s+jyVgfhjvQvPWTp2iSJzbN0RrrJ2lJ8GHFZZnZ37QlgThM/PRGOwqZBI8N4ALTYpE28+XI8pWipAqz8bQoRlhBKX/swYvi1xRRQXWa1wgdMa6DCVLBjm2twPcnAzzu/8z60X47FaX8Pz8ZFOVBray9tZFtvRw7DRYuDhQ4mrdQCd/LxggLLRDwqOgS74MpZrGidxNjqxWnpuorFoM8TxU13x3B6tkoCU6uua9EKAZbX31ihQjep2wRsNBYn0HnumAeFeQXsiycDMtgPgKlv8JukGQr0A6aLv5M1pMcGFgeWnRoPd4f1yTyiMKlTFc2KEVY0ZxS75Kk4nOAVwjNS53ZeXYjuXH/X9+66an0FMNLIoC8m2vG8ZBwltjtWRfugoh25yrWYmLrgTYZmPViE3dE7P2PMC6awYnvWq8y GXuvsd3v jCOblgAOKS/1LhmI5hHW/xbZtigKyx+7Yc1W8zqA5LZQISMs0EPCykl2XCwUgUWxa3w1I4eYkZBO/1q/kHegc+RwcIt40lAOeypk1b2TUzKoNoiwWGEGH61h2EiIUSt22RqjWFOX1pRR+AcsiroN7eW5EpANnqYIyqV11CAkFv9pF0tRRsu8FHD/sojYbrzuRTe/D6y1FKJr20QqQWbltYw7p9bZ3bK/JzAnI2m4+0Q1heEZvZnGXf+0o+lUmgbJRZL0nzLR9MKBpfRlJayL776VOWtPQQj6P7wZf6IEUO0mWbhMB9UBPB7KYgnAIHTFxC6u8Ys476FDuWzQkGofayXM0Tjs4RJE6Ia/eCge/YwvT8J+G7pvRGdvtZNBtoL3ODdJd86z8gL+SXRtPFR2xpP0a9MwXBWWElkp6P3gzr6zBED0C+mNihUrHTcnp9SWWpwJgWtDvvK9OG1cWDEYuu+PM7Fo2c1kkWQWKMGNcPh4NEL26HeyHIoi+kwh5gtv0pB1x3f9p0l6I3rBpCjeNudW9LhASWD444Y/pyH+qNq/xOIZ68rjoX+3wBg== 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 Tue, 13 Jun 2023 at 11:00, Xiaoming Ding (=E4=B8=81=E6=99=93=E6=98=8E) wrote: > > So do we have a conclution about this patch? or need more time to > study the possible risks Please avoid top posting. As already discussed here [1], RLIMIT_MEMLOCK checks have to be implemented if we switch to FOLL_LONGTERM. [1] https://lists.trustedfirmware.org/archives/list/op-tee@lists.trustedfir= mware.org/message/UEOMNYLDFHDFQNLODGCJVFDOQBR723EQ/ -Sumit > > On Tue, 2023-05-23 at 08:25 +0100, Lorenzo Stoakes wrote: > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > > > > > On Mon, May 22, 2023 at 06:54:29PM -0700, John Hubbard wrote: > > > On 5/18/23 06:56, David Hildenbrand wrote: > > > > On 18.05.23 08:08, Sumit Garg wrote: > > > > > On Thu, 18 May 2023 at 09:51, Christoph Hellwig < > > > > > hch@infradead.org> wrote: > > > > > > > > > > > > On Wed, May 17, 2023 at 08:23:33PM +0200, David Hildenbrand > > > > > > wrote: > > > > > > > In general: if user space controls it -> possibly forever > > > > > > > -> long-term. Even > > > > > > > if in most cases it's a short delay: there is no trusting > > > > > > > on user space. > > > > > > > > > > > > > > For example, iouring fixed buffers keep pages pinned until > > > > > > > user space > > > > > > > decides to unregistered the buffers -> long-term. > > > > > > > > > > > > > > Short-term is, for example, something like O_DIRECT where > > > > > > > we pin -> DMA -> > > > > > > > unpin in essentially one operation. > > > > > > > > > > > > Btw, one thing that's been on my mind is that I think we got > > > > > > the > > > > > > polarity on FOLL_LONGTERM wrong. Instead of opting into the > > > > > > long term > > > > > > behavior it really should be the default, with a > > > > > > FOLL_EPHEMERAL flag > > > > > > to opt out of it. And every users of this flag is required > > > > > > to have > > > > > > a comment explaining the life time rules for the pin.. > > > > I couldn't agree more, based on my recent forays into GUP the > > interface > > continues to strike me as odd:- > > > > - FOLL_GET is a wing and a prayer that nothing that > > [folio|page]_maybe_dma_pinned() prevents happens in the brief > > period the > > page is pinned/manipulated. So agree completely with David's > > concept of > > unexporting that and perhaps carefully considering our use of > > it. Obviously the comments around functions like gup_remote() make > > clear > > that 'this page not be what you think it is' but I wonder whether > > many > > callers of GUP _truly_ take that on board. > > > > - FOLL_LONGTERM is entirely optional for PUP and you can just go > > ahead and > > fragment page blocks to your heart's content. Of course this would > > be an > > abuse, but abuses happen. > > > > - With the recent change to PUP/FOLL_LONGTERM disallowing dirty > > tracked > > file-backed mappings we're now really relying on this flag > > indicating a > > _long term_ pin semantically. By defaulting to this being switched > > on, we > > avoid cases of callers who might end up treating the won't > > reclaim/etc. aspect of PUP as all they care about while ignoring > > the > > MIGRATE_MOVABLE aspect. > > > > > > > > I see maybe 10 or 20 call sites today. So it is definitely feasible > > > to add > > > documentation at each, explaining the why it wants a long term pin. > > > > > > > Yeah, my efforts at e.g. dropping vmas has been eye-opening in > > actually > > quite how often a refactoring like this often ends up being more > > straightforward than you might imagine. > > > > > > > > > > > > It does look like a better approach to me given the very nature > > > > > of > > > > > user space pages. > > > > > > > > Yeah, there is a lot of historical baggage. For example, FOLL_GET > > > > should be inaccessible to kernel modules completely at one point, > > > > to be only used by selected core-mm pieces. > > > > > > Yes. When I first mass-converted call sites from gup to pup, I just > > > preserved FOLL_GET behavior in order to keep from changing too much > > > at > > > once. But I agree that that it would be nice to make FOLL_GET an > > > mm internal-only flag like FOLL_PIN. > > > > Very glad you did that work! And totally understandable as to you > > being > > conservative with that, but I think we're at a point where there's > > more > > acceptance of incremental improvements to GUP as a whole. > > > > I have another patch series saved up for _yet more_ changes on this. > > But > > mindful of churn I am trying to space them out... until Jason nudges > > me of > > course :) > > > > > > > > > > > > > Maybe we should even disallow passing in FOLL_LONGTERM as a flag > > > > and only provide functions like pin_user_pages() vs. > > > > pin_user_pages_longterm(). Then, discussions about conditional > > > > flag-setting are no more :) > > > > > > > > ... or even use pin_user_pages_shortterm() vs. pin_user_pages() > > > > ... to make the default be longterm. > > > > > > > > > > Yes, it is true that having most gup flags be internal to mm does > > > tend > > > to avoid some bugs. But it's also a lot of churn. I'm still on the > > > fence > > > as to whether it's really a good move to do this for FOLL_LONGTERM > > > or > > > not. But it's really easy to push me off of fences. :) > > > > *nudge* ;) > > > > > > > > thanks, > > > -- > > > John Hubbard > > > NVIDIA > > > > > > > Looking at non-fast, non-FOLL_LONGTERM PUP callers (forgive me if I > > missed any):- > > > > - pin_user_pages_remote() in process_vm_rw_single_vec() for the > > process_vm_access functionality. > > > > - pin_user_pages_remote() in user_event_enabler_write() in > > kernel/trace/trace_events_user.c. > > > > - pin_user_pages_unlocked() in ivtv_udma_setup() in > > drivers/media/pci/ivtv/ivtv-udma.c and ivtv_yuv_prep_user_dma() in > > ivtv-yuv.c. > > > > And none that actually directly invoke PUP without FOLL_LOGNTERM... > > That > > suggests that we could simply disallow non-FOLL_LONGTERM non-fast PUP > > calls > > altogether and move to pin_user_pages_longterm() [I'm happy to write > > a > > patch series doing this]. > > > > The ivtv callers look like they really actually want FOLL_LONGTERM > > unless > > I'm missing something so we should probably change that too? > > > > I haven't surveyed the fast versions, but I think defaulting to > > FOLL_LONGTERM on them also makes sense. >