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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A64DC433FE for ; Wed, 19 Jan 2022 20:37:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244868AbiASUhm (ORCPT ); Wed, 19 Jan 2022 15:37:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233766AbiASUhl (ORCPT ); Wed, 19 Jan 2022 15:37:41 -0500 Received: from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com [IPv6:2a00:1450:4864:20::12f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 219E4C061574 for ; Wed, 19 Jan 2022 12:37:41 -0800 (PST) Received: by mail-lf1-x12f.google.com with SMTP id m3so13259828lfu.0 for ; Wed, 19 Jan 2022 12:37:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=Vw/hy5bg5C+CCYO+oJtgWYZCGRowuXfpuM1y/pli43kr+OiB3OWzkR6ZyW+nfXxt/s VAn2AjX3mL+nlHsEF9z/pBhrlFgPcsEA0h1sAo1ZC4YcWsIWZ0jeb+wpDXa+U2omlUCF pf/73OVbaBLU61/xMVSHktQI827IhnQn2hI4w8Tkbj/fmE8A6Xntpo4ktyfN/ATLaSyR OmbGO1wuYpDN6JCPJ6apqXqpoeOCB/ZbqWFXSeJZIDyaqZ4S0vxFtOgFaUm3xw+cnlGx I1oFnxocUDYadU9R73MgSd6PbQAJyeaWf6JpFnbLe1rT3w0y4YtI4EsZ5WHxq0hL/Blm wwzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=MpuveFq9I6XMiNEH9wg3gHKo8aRhwiPj37bIV/cjWUWccsAD5KCbMTeu21dqPFTeUU /Vn1QASvXZ7OQFzRTXey6iMXaRyOzopQ+7Az1s6nIhKnvNLq4iye35I+34cPf+Og3SKE L/xMgAUFGOJ9qCfXQ7I+cQf+/lQy5iRp2miGSvU5tPgcFspXLYwj5I13wDwmEZSItkb8 om1dERAAZLRlLi7Pcy4/jOOpzVUDqDa8wgQ9EzCm5UIgx5q7tgzaHUEquzUcb0nttLVp dNdauS3EU+nE7nDV4GRf+Ytkh71WaV0Y2JUSdhoxVitrytWh7g+vq6glEGD3I2LycbPF gnEQ== X-Gm-Message-State: AOAM533hy/KMR1Q4C+kxdin8L8uLm4pyyKVKwoayJ0eqRMtkR5QC6fw0 +IadOMnFgMuaFj4Q1dyT9d9g2E1D12KA+QKc2XMiYg== X-Google-Smtp-Source: ABdhPJwjspQiMNHOBlTaaj+hdTY9hJrpsFobebo94UsQ78o3vrS70VesOp4vSiK9m1G+NqmP7oa+dvJBc2Sn675qSo0= X-Received: by 2002:a05:6512:329b:: with SMTP id p27mr30040874lfe.36.1642624659492; Wed, 19 Jan 2022 12:37:39 -0800 (PST) MIME-Version: 1.0 References: <20220113123406.11520-1-guangming.cao@mediatek.com> <4f88205c1b344aea8608960e2f85b8f4@intel.com> <24157767-dc29-bbdd-5428-d89ecc6b9606@amd.com> <6b8182a1-7cdc-7369-5c34-e6d0c24efcca@amd.com> <82faa62f1bc946cf2f9ee2f7d15c567162238eab.camel@mediatek.com> In-Reply-To: From: John Stultz Date: Wed, 19 Jan 2022 12:37:27 -0800 Message-ID: Subject: Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation To: "Guangming.Cao" Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , "Ruhl, Michael J" , "sumit.semwal@linaro.org" , "linux-arm-kernel@lists.infradead.org" , "wsd_upstream@mediatek.com" , "libo.kang@mediatek.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "yf.wang@mediatek.com" , "linaro-mm-sig@lists.linaro.org" , "linux-mediatek@lists.infradead.org" , "lmark@codeaurora.org" , "benjamin.gaignard@linaro.org" , "bo.song@mediatek.com" , "matthias.bgg@gmail.com" , "labbott@redhat.com" , "mingyuan.ma@mediatek.com" , "jianjiao.zeng@mediatek.com" , "linux-media@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao wrote: > On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > > If the max value is per-heap, why not enforce that value in the > > per-heap allocation function? > > > > Moving the check to the heap alloc to me seems simpler to me than > > adding complexity to the infrastructure to add a heap max_size > > callback. Is there some other use for the callback that you envision? > > > > If you think max the value is per-heap, why not add an optional > callback for dma-heap to solve this issue(prevent consuming too much > time for a doomed to fail allocation), if the dma-heap doesn't have a > special size check, just use the default value(totalram) in dma-heap > framework to do the size check. As the totalram default isn't correct for all heaps (or necessarily even most heaps), so those heaps would need to implement the callback. I'm just not sure adding complexity to the framework to address this is useful. Instead of an additional check in the allocation function, heap implementers will need to assess if the default logic in a framework is correct, and then possibly implement the callback. > Yes, for linux dma-heaps, only system-heap needs it, so adding it in > system heap is the simplest. However, there are many vendor dma-heaps > like system-heap which won't be uploaded to linux codebase, and maybe > have same limitation, all these heaps need to add the same limitation. My worry is that without seeing these vendor heaps, this is a bit of a theoretical concern. We don't have the data on how common this is. I very much hope that vendors can start submitting their heaps upstream (along with drivers that benefit from the heaps). Then we can really assess what makes the most sense for the community maintained code. > I just think it's boring. However, If you think discussing these absent > cases based on current linux code is meaningless, I also agree to it. So, as a rule, the upstream kernel doesn't create/maintain logic to accommodate out of tree code. Now, I agree there is the potential for some duplication in the checks in the allocation logic, but until it affects the upstream kernel, community maintainers can't really make an appropriate evaluation. As a contra-example, if the allocation is some extreme hotpath, adding an extra un-inlinable function pointer traversal for the size callback may actually have a negative impact. This isn't likely but again, if we cannot demonstrate it one way or the other against the upstream tree, we can't figure out what the best solution might be. > So, to summarize, if you still think adding it in system_heap.c is > better, I also agree and I will update the patch to add it in > system_heap.c I think this is the best solution for now. As this is not part of an userland ABI, we can always change it in the future once we see the need. thanks -john 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 99525C433EF for ; Wed, 19 Jan 2022 20:37:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9002710E1AF; Wed, 19 Jan 2022 20:37:42 +0000 (UTC) Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3496110E177 for ; Wed, 19 Jan 2022 20:37:41 +0000 (UTC) Received: by mail-lf1-x133.google.com with SMTP id x22so12949812lfd.10 for ; Wed, 19 Jan 2022 12:37:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=Vw/hy5bg5C+CCYO+oJtgWYZCGRowuXfpuM1y/pli43kr+OiB3OWzkR6ZyW+nfXxt/s VAn2AjX3mL+nlHsEF9z/pBhrlFgPcsEA0h1sAo1ZC4YcWsIWZ0jeb+wpDXa+U2omlUCF pf/73OVbaBLU61/xMVSHktQI827IhnQn2hI4w8Tkbj/fmE8A6Xntpo4ktyfN/ATLaSyR OmbGO1wuYpDN6JCPJ6apqXqpoeOCB/ZbqWFXSeJZIDyaqZ4S0vxFtOgFaUm3xw+cnlGx I1oFnxocUDYadU9R73MgSd6PbQAJyeaWf6JpFnbLe1rT3w0y4YtI4EsZ5WHxq0hL/Blm wwzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=PB/VvIPJd0dlnjQOSEuOZOp1yeSCb4MlEtPUMk/S3NQTq/E0uBpofXr2ZbXjiXt+wb FuJ2P3mL4mFlhtLth8f0pDQWU1TYZuQAbXltixuqCMl0KomLmojtBiz2HP/sNLnv81VX yDECKbvIUzs0lqrAU0lRIZYaroTLWeeTOVPSuCm+w2rOe917e7Ucu2dTjDVtAbX0IvFO l582IYiaPncrLs/MwoWEovnL1vJoojb6wb8evSxKkyufl4AsW1gsJJgDI1rl4CCmQbIz 07JzJjNNTyMtP9NB6z1lwkgXrnxtG7vWl4K7FE7K9LTFvAdBuTKPikTwcwc1ZWqiKqZh jLQA== X-Gm-Message-State: AOAM532E7mvLl7UAbkVOerQXEHxCt4Rx0S26531BRzym6TNPUyrzF8nv feRcJM/5MiLH7eWjWoRTCTytSVkLWyYxLP9RwzvKeA== X-Google-Smtp-Source: ABdhPJwjspQiMNHOBlTaaj+hdTY9hJrpsFobebo94UsQ78o3vrS70VesOp4vSiK9m1G+NqmP7oa+dvJBc2Sn675qSo0= X-Received: by 2002:a05:6512:329b:: with SMTP id p27mr30040874lfe.36.1642624659492; Wed, 19 Jan 2022 12:37:39 -0800 (PST) MIME-Version: 1.0 References: <20220113123406.11520-1-guangming.cao@mediatek.com> <4f88205c1b344aea8608960e2f85b8f4@intel.com> <24157767-dc29-bbdd-5428-d89ecc6b9606@amd.com> <6b8182a1-7cdc-7369-5c34-e6d0c24efcca@amd.com> <82faa62f1bc946cf2f9ee2f7d15c567162238eab.camel@mediatek.com> In-Reply-To: From: John Stultz Date: Wed, 19 Jan 2022 12:37:27 -0800 Message-ID: Subject: Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation To: "Guangming.Cao" Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "jianjiao.zeng@mediatek.com" , "lmark@codeaurora.org" , "wsd_upstream@mediatek.com" , "linux-kernel@vger.kernel.org" , "libo.kang@mediatek.com" , "linaro-mm-sig@lists.linaro.org" , "Ruhl, Michael J" , "yf.wang@mediatek.com" , "linux-mediatek@lists.infradead.org" , "dri-devel@lists.freedesktop.org" , "benjamin.gaignard@linaro.org" , "bo.song@mediatek.com" , "matthias.bgg@gmail.com" , "mingyuan.ma@mediatek.com" , "labbott@redhat.com" , =?UTF-8?Q?Christian_K=C3=B6nig?= , "linux-arm-kernel@lists.infradead.org" , "linux-media@vger.kernel.org" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao wrote: > On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > > If the max value is per-heap, why not enforce that value in the > > per-heap allocation function? > > > > Moving the check to the heap alloc to me seems simpler to me than > > adding complexity to the infrastructure to add a heap max_size > > callback. Is there some other use for the callback that you envision? > > > > If you think max the value is per-heap, why not add an optional > callback for dma-heap to solve this issue(prevent consuming too much > time for a doomed to fail allocation), if the dma-heap doesn't have a > special size check, just use the default value(totalram) in dma-heap > framework to do the size check. As the totalram default isn't correct for all heaps (or necessarily even most heaps), so those heaps would need to implement the callback. I'm just not sure adding complexity to the framework to address this is useful. Instead of an additional check in the allocation function, heap implementers will need to assess if the default logic in a framework is correct, and then possibly implement the callback. > Yes, for linux dma-heaps, only system-heap needs it, so adding it in > system heap is the simplest. However, there are many vendor dma-heaps > like system-heap which won't be uploaded to linux codebase, and maybe > have same limitation, all these heaps need to add the same limitation. My worry is that without seeing these vendor heaps, this is a bit of a theoretical concern. We don't have the data on how common this is. I very much hope that vendors can start submitting their heaps upstream (along with drivers that benefit from the heaps). Then we can really assess what makes the most sense for the community maintained code. > I just think it's boring. However, If you think discussing these absent > cases based on current linux code is meaningless, I also agree to it. So, as a rule, the upstream kernel doesn't create/maintain logic to accommodate out of tree code. Now, I agree there is the potential for some duplication in the checks in the allocation logic, but until it affects the upstream kernel, community maintainers can't really make an appropriate evaluation. As a contra-example, if the allocation is some extreme hotpath, adding an extra un-inlinable function pointer traversal for the size callback may actually have a negative impact. This isn't likely but again, if we cannot demonstrate it one way or the other against the upstream tree, we can't figure out what the best solution might be. > So, to summarize, if you still think adding it in system_heap.c is > better, I also agree and I will update the patch to add it in > system_heap.c I think this is the best solution for now. As this is not part of an userland ABI, we can always change it in the future once we see the need. thanks -john 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D054EC433F5 for ; Wed, 19 Jan 2022 20:38:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=89bIDjo81rojQvg3aUIiTV6y4Fd1rHqAdQeLCU4fXkM=; b=inDVck82hzlBWp VYcCmJdWyf9ZZLVR2uekfU14PwuoUMrUQ/pks3kbgiDkNk5kujZbD6758Uld2EvTvNqc/ZU4CBsjj LH7/lNuvI7ohnDEpzQQQXzAOsimxaWiY2nnrPNcHMzFvewdYyO9oJPox6vnvz2NgC8WmfBVbv2VkO uHoqakMw5qtGtoj+pusPpw1zSkFqf/TsnLJIOffPjRfSvbnohGUUBea4yoCjXoY+03m60jNy79Cq2 3YbvJwQ6LfHZW4zcKQW7i73AHBZqJxrid1Ibg4MELsPA3Vvp9jp0p97/Zk9qnerSneaoIfRTecFMY fuOo1SzSaIsEbdrd5f/g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAHiN-007Kq2-F1; Wed, 19 Jan 2022 20:37:55 +0000 Received: from mail-lf1-x135.google.com ([2a00:1450:4864:20::135]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAHiA-007KlH-0v for linux-mediatek@lists.infradead.org; Wed, 19 Jan 2022 20:37:43 +0000 Received: by mail-lf1-x135.google.com with SMTP id o12so12927896lfu.12 for ; Wed, 19 Jan 2022 12:37:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=Vw/hy5bg5C+CCYO+oJtgWYZCGRowuXfpuM1y/pli43kr+OiB3OWzkR6ZyW+nfXxt/s VAn2AjX3mL+nlHsEF9z/pBhrlFgPcsEA0h1sAo1ZC4YcWsIWZ0jeb+wpDXa+U2omlUCF pf/73OVbaBLU61/xMVSHktQI827IhnQn2hI4w8Tkbj/fmE8A6Xntpo4ktyfN/ATLaSyR OmbGO1wuYpDN6JCPJ6apqXqpoeOCB/ZbqWFXSeJZIDyaqZ4S0vxFtOgFaUm3xw+cnlGx I1oFnxocUDYadU9R73MgSd6PbQAJyeaWf6JpFnbLe1rT3w0y4YtI4EsZ5WHxq0hL/Blm wwzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=nXMWcv7nopz97Cv7dxhTzc0jVsnf+8nHCUbp/BSv2ZDjQFQtwbkRSyiLIsiSKALuNw 3swBeY2L8Fi+R9Ojvczy1dQNVoQMJmLzLqUfbxsOCy50jdZrVdRxBs2lHyrzhY+7I8A/ ph9juDPUbEdivMkC1xGsKRwuL1gBzDHC+TeEZ5kCY5y5Qsrc+dfZOccg90yk9cJoPYyz lSHhPQ0q+zxuQ7sBPWaHJx+mglUy1kXfuKKiTFAuJB5FGmm91jkC5JtS49qilVkmpAeG 9igK93rEi0poejwkfsB2ycAG5EYmoAnenbDtoTEiZ95DIE0xKSBeI6awhpfvuSGGps4e owgw== X-Gm-Message-State: AOAM5327d0c4G/vF8xVPTKTOaTNGoIxv1Qx0kkevWjIhIdY4fr7Yh05V cQ/wY3goIALg6SPv6t6aC7cmAlxDH3Nc0ALAWn6kpg== X-Google-Smtp-Source: ABdhPJwjspQiMNHOBlTaaj+hdTY9hJrpsFobebo94UsQ78o3vrS70VesOp4vSiK9m1G+NqmP7oa+dvJBc2Sn675qSo0= X-Received: by 2002:a05:6512:329b:: with SMTP id p27mr30040874lfe.36.1642624659492; Wed, 19 Jan 2022 12:37:39 -0800 (PST) MIME-Version: 1.0 References: <20220113123406.11520-1-guangming.cao@mediatek.com> <4f88205c1b344aea8608960e2f85b8f4@intel.com> <24157767-dc29-bbdd-5428-d89ecc6b9606@amd.com> <6b8182a1-7cdc-7369-5c34-e6d0c24efcca@amd.com> <82faa62f1bc946cf2f9ee2f7d15c567162238eab.camel@mediatek.com> In-Reply-To: From: John Stultz Date: Wed, 19 Jan 2022 12:37:27 -0800 Message-ID: Subject: Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation To: "Guangming.Cao" Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , "Ruhl, Michael J" , "sumit.semwal@linaro.org" , "linux-arm-kernel@lists.infradead.org" , "wsd_upstream@mediatek.com" , "libo.kang@mediatek.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "yf.wang@mediatek.com" , "linaro-mm-sig@lists.linaro.org" , "linux-mediatek@lists.infradead.org" , "lmark@codeaurora.org" , "benjamin.gaignard@linaro.org" , "bo.song@mediatek.com" , "matthias.bgg@gmail.com" , "labbott@redhat.com" , "mingyuan.ma@mediatek.com" , "jianjiao.zeng@mediatek.com" , "linux-media@vger.kernel.org" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220119_123742_076893_B6B01340 X-CRM114-Status: GOOD ( 29.06 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao wrote: > On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > > If the max value is per-heap, why not enforce that value in the > > per-heap allocation function? > > > > Moving the check to the heap alloc to me seems simpler to me than > > adding complexity to the infrastructure to add a heap max_size > > callback. Is there some other use for the callback that you envision? > > > > If you think max the value is per-heap, why not add an optional > callback for dma-heap to solve this issue(prevent consuming too much > time for a doomed to fail allocation), if the dma-heap doesn't have a > special size check, just use the default value(totalram) in dma-heap > framework to do the size check. As the totalram default isn't correct for all heaps (or necessarily even most heaps), so those heaps would need to implement the callback. I'm just not sure adding complexity to the framework to address this is useful. Instead of an additional check in the allocation function, heap implementers will need to assess if the default logic in a framework is correct, and then possibly implement the callback. > Yes, for linux dma-heaps, only system-heap needs it, so adding it in > system heap is the simplest. However, there are many vendor dma-heaps > like system-heap which won't be uploaded to linux codebase, and maybe > have same limitation, all these heaps need to add the same limitation. My worry is that without seeing these vendor heaps, this is a bit of a theoretical concern. We don't have the data on how common this is. I very much hope that vendors can start submitting their heaps upstream (along with drivers that benefit from the heaps). Then we can really assess what makes the most sense for the community maintained code. > I just think it's boring. However, If you think discussing these absent > cases based on current linux code is meaningless, I also agree to it. So, as a rule, the upstream kernel doesn't create/maintain logic to accommodate out of tree code. Now, I agree there is the potential for some duplication in the checks in the allocation logic, but until it affects the upstream kernel, community maintainers can't really make an appropriate evaluation. As a contra-example, if the allocation is some extreme hotpath, adding an extra un-inlinable function pointer traversal for the size callback may actually have a negative impact. This isn't likely but again, if we cannot demonstrate it one way or the other against the upstream tree, we can't figure out what the best solution might be. > So, to summarize, if you still think adding it in system_heap.c is > better, I also agree and I will update the patch to add it in > system_heap.c I think this is the best solution for now. As this is not part of an userland ABI, we can always change it in the future once we see the need. thanks -john _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A373C433EF for ; Wed, 19 Jan 2022 20:39:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3vAL/ek7Dbt/UxGRN0J0fdd+dn0gJQoyjTg9NhepDOo=; b=pwQFjqYkOIHRJX b+y0xwSOFlGsZX2Srsxk/Oe0ELnzTa0+uQdW6ygAw44Z2zP+MstMZdcs/bJj44DasU4TdZkuwJ4Ts GIE9/2mvIsHfzUmMeEFwouidZSHFFCzlizZoHQk/LrB0SmQfDpQo2342qIhPeB4L5u2amCcwfYl6q DaB6GcRnCkPxl7l9Wp8hpjsVt4KBhJhWesawiYden8U5ydBYRpS7qAc7hs30mgHYqiCe7udwaph6U tmZJrdBkoTf62BN0pJuqTsqt/QLcEGG8OPcuu/xGmY14nuKm5pkJaRCCfTI6/L7ckmqRys/nynNVh iL76ue2ZaWzYW/UwnRXQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAHiD-007KnW-LW; Wed, 19 Jan 2022 20:37:45 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAHi9-007KlG-TU for linux-arm-kernel@lists.infradead.org; Wed, 19 Jan 2022 20:37:43 +0000 Received: by mail-lf1-x12b.google.com with SMTP id p27so13122010lfa.1 for ; Wed, 19 Jan 2022 12:37:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=Vw/hy5bg5C+CCYO+oJtgWYZCGRowuXfpuM1y/pli43kr+OiB3OWzkR6ZyW+nfXxt/s VAn2AjX3mL+nlHsEF9z/pBhrlFgPcsEA0h1sAo1ZC4YcWsIWZ0jeb+wpDXa+U2omlUCF pf/73OVbaBLU61/xMVSHktQI827IhnQn2hI4w8Tkbj/fmE8A6Xntpo4ktyfN/ATLaSyR OmbGO1wuYpDN6JCPJ6apqXqpoeOCB/ZbqWFXSeJZIDyaqZ4S0vxFtOgFaUm3xw+cnlGx I1oFnxocUDYadU9R73MgSd6PbQAJyeaWf6JpFnbLe1rT3w0y4YtI4EsZ5WHxq0hL/Blm wwzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SVNoxQMp637bgUGyLKUQMTT6jCUfGMhBKrAhjOzC1jA=; b=LGm5VDwhonIfaWHKmlf3EgTzyJsj0N4vh7hJ59nQvOXLHS/R6ew7UFdcCKnfyDcyt0 TYxhvfeWcLm0ECr3bTLYlJD1EotoolGPkoxauWaijTExhxxMsek+5XxalletAmR4Kw02 Zy0ufZsl18i33ZoEpXLLKxpWRqLS8Hxj3sPGtdLfL4oSxsUyZqfgMZjiUSYs9h8SZZFQ av0i+D1Spp8MsVibCKE3XpaW/bFiCA9gc2l++IGZVqsDwONYUudaqRLmUixGsDxeE36y HFEjnE1xIVi55UgzgpKTRNQLV44JdNQIW/g3mU8tz34FFaUdBv6ebTYeXVLG7tkx/3kh UFbg== X-Gm-Message-State: AOAM533IgcDktZ0HvhQJ9Qua7OfHe3FAJBQg+DkqcyEPAhpWhaBdBmym jldzFBJ7Nz2up+oILLg2Z5j4FZOppY6+cxMfSUfEdg== X-Google-Smtp-Source: ABdhPJwjspQiMNHOBlTaaj+hdTY9hJrpsFobebo94UsQ78o3vrS70VesOp4vSiK9m1G+NqmP7oa+dvJBc2Sn675qSo0= X-Received: by 2002:a05:6512:329b:: with SMTP id p27mr30040874lfe.36.1642624659492; Wed, 19 Jan 2022 12:37:39 -0800 (PST) MIME-Version: 1.0 References: <20220113123406.11520-1-guangming.cao@mediatek.com> <4f88205c1b344aea8608960e2f85b8f4@intel.com> <24157767-dc29-bbdd-5428-d89ecc6b9606@amd.com> <6b8182a1-7cdc-7369-5c34-e6d0c24efcca@amd.com> <82faa62f1bc946cf2f9ee2f7d15c567162238eab.camel@mediatek.com> In-Reply-To: From: John Stultz Date: Wed, 19 Jan 2022 12:37:27 -0800 Message-ID: Subject: Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation To: "Guangming.Cao" Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , "Ruhl, Michael J" , "sumit.semwal@linaro.org" , "linux-arm-kernel@lists.infradead.org" , "wsd_upstream@mediatek.com" , "libo.kang@mediatek.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "yf.wang@mediatek.com" , "linaro-mm-sig@lists.linaro.org" , "linux-mediatek@lists.infradead.org" , "lmark@codeaurora.org" , "benjamin.gaignard@linaro.org" , "bo.song@mediatek.com" , "matthias.bgg@gmail.com" , "labbott@redhat.com" , "mingyuan.ma@mediatek.com" , "jianjiao.zeng@mediatek.com" , "linux-media@vger.kernel.org" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220119_123741_980890_1D3D0A45 X-CRM114-Status: GOOD ( 30.37 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao wrote: > On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > > If the max value is per-heap, why not enforce that value in the > > per-heap allocation function? > > > > Moving the check to the heap alloc to me seems simpler to me than > > adding complexity to the infrastructure to add a heap max_size > > callback. Is there some other use for the callback that you envision? > > > > If you think max the value is per-heap, why not add an optional > callback for dma-heap to solve this issue(prevent consuming too much > time for a doomed to fail allocation), if the dma-heap doesn't have a > special size check, just use the default value(totalram) in dma-heap > framework to do the size check. As the totalram default isn't correct for all heaps (or necessarily even most heaps), so those heaps would need to implement the callback. I'm just not sure adding complexity to the framework to address this is useful. Instead of an additional check in the allocation function, heap implementers will need to assess if the default logic in a framework is correct, and then possibly implement the callback. > Yes, for linux dma-heaps, only system-heap needs it, so adding it in > system heap is the simplest. However, there are many vendor dma-heaps > like system-heap which won't be uploaded to linux codebase, and maybe > have same limitation, all these heaps need to add the same limitation. My worry is that without seeing these vendor heaps, this is a bit of a theoretical concern. We don't have the data on how common this is. I very much hope that vendors can start submitting their heaps upstream (along with drivers that benefit from the heaps). Then we can really assess what makes the most sense for the community maintained code. > I just think it's boring. However, If you think discussing these absent > cases based on current linux code is meaningless, I also agree to it. So, as a rule, the upstream kernel doesn't create/maintain logic to accommodate out of tree code. Now, I agree there is the potential for some duplication in the checks in the allocation logic, but until it affects the upstream kernel, community maintainers can't really make an appropriate evaluation. As a contra-example, if the allocation is some extreme hotpath, adding an extra un-inlinable function pointer traversal for the size callback may actually have a negative impact. This isn't likely but again, if we cannot demonstrate it one way or the other against the upstream tree, we can't figure out what the best solution might be. > So, to summarize, if you still think adding it in system_heap.c is > better, I also agree and I will update the patch to add it in > system_heap.c I think this is the best solution for now. As this is not part of an userland ABI, we can always change it in the future once we see the need. thanks -john _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel