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 0765DC54E58 for ; Mon, 18 Mar 2024 14:25:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 46FDB8E0008; Mon, 18 Mar 2024 10:25:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 41FCB8E0005; Mon, 18 Mar 2024 10:25:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2C02A8E0008; Mon, 18 Mar 2024 10:25:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1AAFC8E0005 for ; Mon, 18 Mar 2024 10:25:34 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B28681C0345 for ; Mon, 18 Mar 2024 14:25:33 +0000 (UTC) X-FDA: 81910382946.24.03CF5C8 Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) by imf08.hostedemail.com (Postfix) with ESMTP id 9AB53160029 for ; Mon, 18 Mar 2024 14:25:29 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=cXnmnBDr; dmarc=none; spf=pass (imf08.hostedemail.com: domain of mcanal@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=mcanal@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710771932; 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=+/9B0iwjGyUjsQimhRpoKXK++f72S5NiH3JNuLyOpx4=; b=4tOBAeEZKcGmsu3GWIclWI+UzVTAV+ZyEu8DEUXJoAoWl3d0SZ6ni0u7DFDS5+j524NxCM zyR2O4LNhDDhk4d5uoiw1Ov23F8Pe+sYcmzl3RgT8gpimYLBI6wNL7lQWD/yjv56JrwcCu lM/2KTqWtierdcfwb6yURD3iR2OxAL4= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=cXnmnBDr; dmarc=none; spf=pass (imf08.hostedemail.com: domain of mcanal@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=mcanal@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710771932; a=rsa-sha256; cv=none; b=Va+l7/NevMnO+AdbiZGgSbWVzOft8d+c7iB9F75pvTqVbGejRh/mGqaDwkm0Is2G1CkJwc P6AgpmA/+sWWwbeEeb+9Kf7hEBYZggLu7dT2bK8rM9R8ktygz6ZmKd29NPfC5rpDxru+7T gwGWobNdMtbC3d9LCuB13rIhSJHt4cY= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=+/9B0iwjGyUjsQimhRpoKXK++f72S5NiH3JNuLyOpx4=; b=cXnmnBDr2WuQE1EQceU6QtbsZr ehuIT+Hu3OD6EOAmHbHvsMF5U1iLOVS/+mgk+y39nze+ooniNJdMu6ov+q7XVzDSWdAqlLjKFWHrL 4f1ux1t34AyMHBYLQEB26ID53nREENBN0mcRJniIf4lsOBp5a0HWGYarUW7bt/eyM+9dLlbiGLK4o 7PveI9VXbvOUL1w81PBKyOaVA4UF4Csz2nRYN9JftjrzsvPV323eoASXiP2napyLg2nWfcs+Pfj1Q A8pdloapLzdM4NQxO1m1MDgeZ5PWDhFVmcWYJlfHju9x1lt/etMDBGdOzf0PzwZgi46ZoZHrsyoho jtArlL/A==; Received: from [143.107.231.30] (helo=[172.26.121.144]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1rmDup-00ByqM-P8; Mon, 18 Mar 2024 15:24:40 +0100 Message-ID: <00a4aa2e-e279-48ff-bc1f-8a72ef5b4491@igalia.com> Date: Mon, 18 Mar 2024 11:24:25 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/5] drm/gem: Add a mountpoint parameter to drm_gem_object_init() Content-Language: en-US To: =?UTF-8?Q?Christian_K=C3=B6nig?= , Tvrtko Ursulin , Melissa Wen , Iago Toral , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter Cc: dri-devel@lists.freedesktop.org, kernel-dev@igalia.com, Russell King , Lucas Stach , Christian Gmeiner , Inki Dae , Seung-Woo Kim , Kyungmin Park , Krzysztof Kozlowski , Alim Akhtar , Patrik Jakobsson , Sui Jingfeng , Chun-Kuang Hu , Philipp Zabel , Matthias Brugger , AngeloGioacchino Del Regno , Rob Clark , Abhinav Kumar , Dmitry Baryshkov , Sean Paul , Marijn Suijten , Karol Herbst , Lyude Paul , Danilo Krummrich , Tomi Valkeinen , Gerd Hoffmann , Sandy Huang , =?UTF-8?Q?Heiko_St=C3=BCbner?= , Andy Yan , Thierry Reding , Mikko Perttunen , Jonathan Hunter , Huang Rui , Oleksandr Andrushchenko , Karolina Stolarek , Andi Shyti , Hugh Dickins , linux-mm@kvack.org References: <20240311100959.205545-1-mcanal@igalia.com> <20240311100959.205545-3-mcanal@igalia.com> <30a7f20b-1f2c-41cb-b193-03429c160b63@igalia.com> <3a5d07c0-120f-47f9-a35a-31f3bfcc9330@amd.com> <07885e3b-ee7b-456b-9fad-17d9009a4cb7@igalia.com> <69576e6d-9704-42b9-905f-289f9f9017b9@amd.com> <9cfb7f83-2d76-4e8d-9052-5975da71e0dc@amd.com> <2118492c-f223-41e9-8c1e-3c03d976301e@igalia.com> <68452c6c-12d7-4e60-9598-369acc9fc360@amd.com> <5f1a758b-b4f6-4fc3-b16b-29d731415636@amd.com> From: =?UTF-8?Q?Ma=C3=ADra_Canal?= In-Reply-To: <5f1a758b-b4f6-4fc3-b16b-29d731415636@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: 8r5dgm4jux8j3ed8dr7adskm7asq6ikb X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 9AB53160029 X-HE-Tag: 1710771929-836428 X-HE-Meta: U2FsdGVkX1+eZqXulmr2PwN7YpfZ1mX0sRIxVM/SmSiDQxPkBOR7LCsWxg9XsXai0mUzVtNCutySBJaZ6zvVbtFY7dwPUsOaCAg9bQ5XsGYKPlqw3YqPewkYl/oyTSkKnA0sziiF+yPTYS5LX1OZSqgSZxX4Yl7zcWr/WlXKEH5a3VPxjvaNSAT07HGWOdp1Vh/jAD03ghPHKDPdZdpJF4pLS0m9mMfSXJRFOaFzvvfqAgKLX4BF5yZz+lk+dI1HqwOKvf9MI1eXj0LifPVj6twh+BVdleBiAdmyIHSqRTWaruXXvxYBlZK+Cq4lm0mgEg6oWNM3wPK4XBs8CNgaR2LxKuvD6XmoPc4FYq+ZbDX/7Cxfdskjwsc2E9lknM9c4rTxbQu/ZXTCUcxctEF0G+HKEAIze6tpoZw5Wrpz3d9HbkSjUa5NG32s/WjS0ICa0Vw070zQf6tsSZnVqVfP0J2c6+f6x++QibBrUTC6CJ17caEFpNQkNBVlBGE8ji6g883IjdrkhPvaEPvBlbtVzgEL2QGNMl53JC9ghtITJnISbZRrZSkWEYxmdMPpopk9H8iSgLXVt7R/PBKMs+k8wfEqnWUwPUfR2KJK5NfRct3pucaMFFdVATO5QAvo/AexW+v/pQdFBQt2KmQuY4zb6B37F3VfCe4Jm3TB68zJz7kfCy1ukiGksia01zBSbwUrONJEthU2WkSaC1EOjKdull/EDILBVa4fMR4dxRzg0ki2ZXS4Vv58jyN8+iHW4Ua0cNgViiZM/x1bzsX92JfmzD3XjjIC5CRK9t/Ymh07R7vwG8nwI+KFT75hLcvFM2D44WTop+ysT5ZA8EWx+Q4FyM+thDRMtQBfRNxl+45q1fst5BPIvop2JVUEJYkhZ39nBPdiWLTqd8tow37a8tL7ljA2BK+SLLrKoq5fE+VsWxjHAHDdsV3PqCl8IBCyHQivuPwFAR+jyK3MXa97q1F M4uuzVCD jYm3BHHhOjVMVIaOdVC1EURo2PSgAA0fALtYBWs4StRxaRr4/b/2oFG9tqzmkmgQ5StoajOknvbvu08u5LGN4/H1n7y3U4DflMrPDWpnF1Jv2ZB+0p1tVtpF1xUlf3smHJFXUPis7DhZOUvke2J1ctedbzEJu2v8HhZG4AaPzLQlBONg+U571vFlN3bp5Ds8wU8vVqHV+3AaHXX2SzcsliKX4o2U1W2jYi7JZMT+3Nyyiti3qJrfpm+PlmtHcuvGJptp14UfEeOxK0gh8rqspTN6mmVMVQwjD0NMDyYaib7d001KqcX/7p+Pv/meDja82XMhhe8XgwoniXUVsEJVQdeYKvzYa5vCzEagsy9WTkKaEJvxE04rY8X2pBFYDgpXVASO1FtqmdFgkXKtUiJB3gwIKd2lPupiIz/s5PE+k0dY++Xs= 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: Not that the CC list wasn't big enough, but I'm adding MM folks in the CC list. On 3/18/24 11:04, Christian König wrote: > Am 18.03.24 um 14:28 schrieb Maíra Canal: >> Hi Christian, >> >> On 3/18/24 10:10, Christian König wrote: >>> Am 18.03.24 um 13:42 schrieb Maíra Canal: >>>> Hi Christian, >>>> >>>> On 3/12/24 10:48, Christian König wrote: >>>>> Am 12.03.24 um 14:09 schrieb Tvrtko Ursulin: >>>>>> >>>>>> On 12/03/2024 10:37, Christian König wrote: >>>>>>> Am 12.03.24 um 11:31 schrieb Tvrtko Ursulin: >>>>>>>> >>>>>>>> On 12/03/2024 10:23, Christian König wrote: >>>>>>>>> Am 12.03.24 um 10:30 schrieb Tvrtko Ursulin: >>>>>>>>>> >>>>>>>>>> On 12/03/2024 08:59, Christian König wrote: >>>>>>>>>>> Am 12.03.24 um 09:51 schrieb Tvrtko Ursulin: >>>>>>>>>>>> >>>>>>>>>>>> Hi Maira, >>>>>>>>>>>> >>>>>>>>>>>> On 11/03/2024 10:05, Maíra Canal wrote: >>>>>>>>>>>>> For some applications, such as using huge pages, we might >>>>>>>>>>>>> want to have a >>>>>>>>>>>>> different mountpoint, for which we pass in mount flags that >>>>>>>>>>>>> better match >>>>>>>>>>>>> our usecase. >>>>>>>>>>>>> >>>>>>>>>>>>> Therefore, add a new parameter to drm_gem_object_init() >>>>>>>>>>>>> that allow us to >>>>>>>>>>>>> define the tmpfs mountpoint where the GEM object will be >>>>>>>>>>>>> created. If >>>>>>>>>>>>> this parameter is NULL, then we fallback to >>>>>>>>>>>>> shmem_file_setup(). >>>>>>>>>>>> >>>>>>>>>>>> One strategy for reducing churn, and so the number of >>>>>>>>>>>> drivers this patch touches, could be to add a lower level >>>>>>>>>>>> drm_gem_object_init() (which takes vfsmount, call it >>>>>>>>>>>> __drm_gem_object_init(), or drm__gem_object_init_mnt(), and >>>>>>>>>>>> make drm_gem_object_init() call that one with a NULL argument. >>>>>>>>>>> >>>>>>>>>>> I would even go a step further into the other direction. The >>>>>>>>>>> shmem backed GEM object is just some special handling as far >>>>>>>>>>> as I can see. >>>>>>>>>>> >>>>>>>>>>> So I would rather suggest to rename all drm_gem_* function >>>>>>>>>>> which only deal with the shmem backed GEM object into >>>>>>>>>>> drm_gem_shmem_*. >>>>>>>>>> >>>>>>>>>> That makes sense although it would be very churny. I at least >>>>>>>>>> would be on the fence regarding the cost vs benefit. >>>>>>>>> >>>>>>>>> Yeah, it should clearly not be part of this patch here. >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Also the explanation why a different mount point helps with >>>>>>>>>>> something isn't very satisfying. >>>>>>>>>> >>>>>>>>>> Not satisfying as you think it is not detailed enough to say >>>>>>>>>> driver wants to use huge pages for performance? Or not >>>>>>>>>> satisying as you question why huge pages would help? >>>>>>>>> >>>>>>>>> That huge pages are beneficial is clear to me, but I'm missing >>>>>>>>> the connection why a different mount point helps with using >>>>>>>>> huge pages. >>>>>>>> >>>>>>>> Ah right, same as in i915, one needs to mount a tmpfs instance >>>>>>>> passing huge=within_size or huge=always option. Default is >>>>>>>> 'never', see man 5 tmpfs. >>>>>>> >>>>>>> Thanks for the explanation, I wasn't aware of that. >>>>>>> >>>>>>> Mhm, shouldn't we always use huge pages? Is there a reason for a >>>>>>> DRM device to not use huge pages with the shmem backend? >>>>>> >>>>>> AFAIU, according to b901bb89324a ("drm/i915/gemfs: enable THP"), >>>>>> back then the understanding was within_size may overallocate, >>>>>> meaning there would be some space wastage, until the memory >>>>>> pressure makes the thp code split the trailing huge page. I >>>>>> haven't checked if that still applies. >>>>>> >>>>>> Other than that I don't know if some drivers/platforms could have >>>>>> problems if they have some limitations or hardcoded assumptions >>>>>> when they iterate the sg list. >>>>> >>>>> Yeah, that was the whole point behind my question. As far as I can >>>>> see this isn't driver specific, but platform specific. >>>>> >>>>> I might be wrong here, but I think we should then probably not have >>>>> that handling in each individual driver, but rather centralized in >>>>> the DRM code. >>>> >>>> I don't see a point in enabling THP for all shmem drivers. A huge page >>>> is only useful if the driver is going to use it. On V3D, for example, >>>> I only need huge pages because I need the memory contiguously allocated >>>> to implement Super Pages. Otherwise, if we don't have the Super Pages >>>> support implemented in the driver, I would be creating memory pressure >>>> without any performance gain. >>> >>> Well that's the point I'm disagreeing with. THP doesn't seem to >>> create much extra memory pressure for this use case. >>> >>> As far as I can see background for the option is that files in tmpfs >>> usually have a varying size, so it usually isn't beneficial to >>> allocate a huge page just to find that the shmem file is much smaller >>> than what's needed. >>> >>> But GEM objects have a fixed size. So we of hand knew if we need 4KiB >>> or 1GiB and can therefore directly allocate huge pages if they are >>> available and object large enough to back them with. >>> >>> If the memory pressure is so high that we don't have huge pages >>> available the shmem code falls back to standard pages anyway. >> >> The matter is: how do we define the point where the memory pressure is >> high? > > Well as driver developers/maintainers we simply don't do that. This is > the job of the shmem code. > >> For example, notice that in this implementation of Super Pages >> for the V3D driver, I only use a Super Page if the BO is bigger than >> 2MB. I'm doing that because the Raspberry Pi only has 4GB of RAM >> available for the GPU. If I created huge pages for every BO allocation >> (and initially, I tried that), I would end up with hangs in some >> applications. > > Yeah, that is what I meant with the trivial optimisation to the shmem > code. Essentially when you have 2MiB+4KiB as BO size then the shmem code > should use a 2MiB and a 4KiB page to back them, but what it currently > does is to use two 2MiB pages and then split up the second page when it > find that it isn't needed. > > That is wasteful and leads to fragmentation, but as soon as we stop > doing that we should be fine to enable it unconditionally for all drivers. I see your point, but I believe that it would be tangent to the goal of this series. As you mentioned, currently, we have a lot of memory fragmentation when using THP and while we don't solve that (at the tmpfs level), I believe we could move on with the current implementation (with improvements proposed by Tvrtko). Best Regards, - Maíra > > TTM does essentially the same thing for years. > > Regards, > Christian. > >> >> >> At least, for V3D, I wouldn't like to see THP being used for all the >> allocations. But, we have maintainers of other drivers in the CC. >> >> Best Regards, >> - Maíra >> >>> >>> So THP is almost always beneficial for GEM even if the driver doesn't >>> actually need it. The only potential case I can think of which might >>> not be handled gracefully is the tail pages, e.g. huge + 4kib. >>> >>> But that is trivial to optimize in the shmem code when the final size >>> of the file is known beforehand. >>> >>> Regards, >>> Christian. >>> >>>> >>>> Best Regards, >>>> - Maíra >>>> >>>>> >>>>> Regards, >>>>> Christian. >>>>> >>>>> >>>>>> >>>>>> Te Cc is plenty large so perhaps someone else will have additional >>>>>> information. :) >>>>>> >>>>>> Regards, >>>>>> >>>>>> Tvrtko >>>>>> >>>>>>> >>>>>>> I mean it would make this patch here even smaller. >>>>>>> >>>>>>> Regards, >>>>>>> Christian. >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Regards, >>>>>>>> >>>>>>>> Tvrtko >>>>>>> >>>>> >>> >