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 BFABCC74A5B for ; Wed, 29 Mar 2023 16:46:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1750B6B0072; Wed, 29 Mar 2023 12:46:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 124E36B0074; Wed, 29 Mar 2023 12:46:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F2EAF6B0075; Wed, 29 Mar 2023 12:46:09 -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 E41576B0072 for ; Wed, 29 Mar 2023 12:46:09 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9EF3FA06E5 for ; Wed, 29 Mar 2023 16:46:09 +0000 (UTC) X-FDA: 80622513258.26.90FB88A Received: from mail-yw1-f172.google.com (mail-yw1-f172.google.com [209.85.128.172]) by imf23.hostedemail.com (Postfix) with ESMTP id 95FDE14000C for ; Wed, 29 Mar 2023 16:46:07 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="iDGYs//g"; spf=pass (imf23.hostedemail.com: domain of tjmercier@google.com designates 209.85.128.172 as permitted sender) smtp.mailfrom=tjmercier@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680108367; 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=PyYGJHwbzUCXb4y32Pa60ou/Hi+y+ZvE44Fz2eCrQLQ=; b=n2czyaDmF6ckjPZGdiNkxIC8WtC3LiTCtR9EjaPFwSIMUtOk8RxGrelAA3gn3L6o9H3PNb kZc3E2fAJOawb1qBg8dxB9K4GStEd0UPgGJO+a6b6IEi8/zctUaj73j5VHGTt+8HHQ2jN7 7qInzbpi5PTFFw26V/1FkoBrygAY+4Q= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="iDGYs//g"; spf=pass (imf23.hostedemail.com: domain of tjmercier@google.com designates 209.85.128.172 as permitted sender) smtp.mailfrom=tjmercier@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680108367; a=rsa-sha256; cv=none; b=3vM6/VuCbxYVo2HlFxQQbXmrPpvFEAQZw8OfVXQu6XybgSs0eHLimqvFtGbIVrxSJliPpK iCqWYpbeZmnZXL9UTQtg7u2yI2TYK3WxNSvKSDmwI3Zfz/hbA19qMQLrts4NnckcckAjBA FcVcCBDvRRSn/b4kEOldSBw9dmdvy4A= Received: by mail-yw1-f172.google.com with SMTP id 00721157ae682-5445009c26bso303610517b3.8 for ; Wed, 29 Mar 2023 09:46:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680108366; 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=PyYGJHwbzUCXb4y32Pa60ou/Hi+y+ZvE44Fz2eCrQLQ=; b=iDGYs//gI2i/1fvcvqQRE2YI543Sc2+B+2QnklKYVuLUjtRJMt2o3Y00AZCjgFWTZM 56FWZS7UyGDgHbcKVXF5qvX6xgXmqXLrx+5QMvI92ckqXs8Lnk29jBJNkc0UOw1TRTsm eXTl45FH6e0rDDgoYKFk1I9b0GPTl/dRfb1XzhLt0AqKD2UnqHawwxmiY9EV5eQQFeCC tl0YKv99LFnb9DPRve4MVjIc8yzLTu115MSGmhGjiV0Sqxj7hcvcuEzZYCCoTOb58Ht4 q6JyyvonRntR578MnkzpMvGWTCIexQ1kRWjfjAUitjyrw/eOLtxrguF64o6Q/ZHJ8B6e ZYhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680108366; 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=PyYGJHwbzUCXb4y32Pa60ou/Hi+y+ZvE44Fz2eCrQLQ=; b=HNtgLcc1Ee1sOR4jZGRBJE6aqapii1QszetyAts8nCOAeCMEEH0yU/SIZUeczYZcvv C95Dz8zIOuvRtnAflA149FBhS7WE8X4iAiR0aiPQHcqbwf4iHAMVv/pti8qTu9Og3e8a tdw/8lDwEAhK1nzLuAf11a6knmwl3N9GLdcYH/xVVdXw4aOYOE53Ydc0NpwhDbKmsheH yt6zyAcch0IYr7QONUMHM70D66puDtLePKE5e4qR0n3Of7oZvWrwQV4bniFQfxGa64rA fEjq4hCkkzRtZd5aDkzpylm5OysSc7oF7SDdeqw6nj+asil16mnMqvlb6O4g2yPckL+g 1IaA== X-Gm-Message-State: AAQBX9fFrfUK7UuPrgZBMKZKbLW46pDsTYXKwrFmSsjqDGmGVl+2ypzf 6sQlV6EUKVdyQc/5J23L5Ax6aNZjCzT0imHlTZfQ4A== X-Google-Smtp-Source: AKy350aIj9rMEyRSpwht5dqjJJGGj34ioNUDhMROVb71O9JE5qD2Kdevyi040wyXqyVUlTqEGIXnQigg1Ugi71u5cdU= X-Received: by 2002:a0d:ec48:0:b0:545:f7cc:f30 with SMTP id r8-20020a0dec48000000b00545f7cc0f30mr5126792ywn.0.1680108366530; Wed, 29 Mar 2023 09:46:06 -0700 (PDT) MIME-Version: 1.0 References: <20230328125818.5574-1-jaewon31.kim@samsung.com> <20230329031302epcms1p6afc9d9d8e92db6a39c29044606d21afc@epcms1p6> In-Reply-To: <20230329031302epcms1p6afc9d9d8e92db6a39c29044606d21afc@epcms1p6> From: "T.J. Mercier" Date: Wed, 29 Mar 2023 09:45:55 -0700 Message-ID: Subject: Re: [PATCH] dma-buf/heaps: c9e8440eca61 staging: ion: Fix overflow and list bugs in system heap: To: jaewon31.kim@samsung.com Cc: "jstultz@google.com" , "sumit.semwal@linaro.org" , "daniel.vetter@ffwll.ch" , "akpm@linux-foundation.org" , "hannes@cmpxchg.org" , "mhocko@kernel.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "jaewon31.kim@gmail.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 4jcot46jabhtdrgm3w35bew1gs1xtdnj X-Rspam-User: X-Rspamd-Queue-Id: 95FDE14000C X-Rspamd-Server: rspam06 X-HE-Tag: 1680108367-112414 X-HE-Meta: U2FsdGVkX1+Aa4zIiTu6JN/odoTVKmux1l4G+jjfc5Gh4SLyqeW5USE+hUh6vCaVHpFs8jymbKuRpHMp2QqRZ2Nh7ghgF5aE8LZobVwah0M34FTbZCMBeKM+toLS+DOFWRUuF/2Z4QJiS4pG8I2oA8wVQYCBYb3XxJWyPM007rPwHVPThlL19CEvK0zCfoJ7c0RE7llQkOoFP3mM8XGOZGEkevgevsm/jUCwYF/lLzzyf30IiHGq2bo9/qln7uDYppx3fp3S8ZrPgCso8UY7RiUAHXd+DNCPb6wLbavn+4r+ZSXUs7Nzxcc8PCtSWCz3nExsLbjA1Rq3nsbLaiYRCaKnfb4yBj+mDo/3/8Z4xUSvGNdSPDl5HhovPuBT8AIjz+AS+hOz6JdLlq4sG45fnkQezlpzHUEbtbNZoWo1OZ43Qft17DKfSmGbVdRBiu5qVtRpBOhFh0AWZ2+FdY1sYBv5+D8CXdLtRz1XbRMlRC0RK4UG+kxN6CsbUaRFiy+v2OTABYD/I7VBj0GxRmLlOxi+Lb1lyROC+qF8tfkF+128barnavXvQauTPACLwiIDXEJpcXBoX0d48K1ZGLjc8ZcfZ4ArERQhNeFhhqRLnqMAzTMukYDZCnCxNjReXokAmLMBixyBHqO6YFqK47bOFS4vF4GdfxkI9BKM8omimSfjN1DaNGWMEQb5GFfdMkDzBGkXsp0jBOmLxqQZHTDolJODtOEuci/kD7qfZleO9KAPSKYeREAiaVb5HbsC3KaBTgeSpdruuL6fQFQqR3LZ7DVEcoH4MnK2fPtaT40kjQKV5kIXCeccozBSEfqoIlZ+Q4e3hmH7QjbyVboQzu/aQ3jK4vb6XxPGyZyXbOvSWPXKiYKFHeY3JwHJzc1npJxwF2m6CynUsDBPui67EdRdz2HmyQOhGxXK2/Pjv2StsYE0P1qcehHz2Of0r+scTUK47XrBjXjBxW0AG5gyO17 Jts+iKra vnAQN2KG19UsMS7iFDiypQa+DGDnoEZp7PPlbI1UtJCtYOgtWAxCqmxe3sDfWWOPYjIk4Cf4OGdUxq+1DoUIz6nWeCF5avIxnGhgunZQIenzNY1Mn7+qTK9gAIdJOcOSjL0VQ0OpGSFZk8bigUSoRrAE4O3W8lICSYhrbodjc1pDFUUdtfwaW6yAN/U9H1QTIH0orJxNLKLfOA3t+Rs+n/DFbBYBFAGFJZvvuwRtvP1lO2UtuZx2CUGI+QUP7/UkUyptjNUCdgCkGlZDmf6qlcIguhsaUDiOVK1XJgy5k5E3C0lQo8guPCdVegGPL9H17PnXxv8IjQhqxceA7wPxuM8+A1DPV3uEoLv9EDPbXElwTvjAhlamoJLwcCBE8IzKRh39EI6wz5jgydvmWI3NsTIcJGQ== 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, Mar 28, 2023 at 8:13=E2=80=AFPM Jaewon Kim wrote: > > >On Tue, Mar 28, 2023 at 5:58?AM Jaewon Kim wr= ote: > >> > >> Normal free:212600kB min:7664kB low:57100kB high:106536kB > >> reserved_highatomic:4096KB active_anon:276kB inactive_anon:180kB > >> active_file:1200kB inactive_file:0kB unevictable:2932kB > >> writepending:0kB present:4109312kB managed:3689488kB mlocked:2932kB > >> pagetables:13600kB bounce:0kB free_pcp:0kB local_pcp:0kB > >> free_cma:200844kB > >> Out of memory and no killable processes... > >> Kernel panic - not syncing: System is deadlocked on memory > >> > >> An OoM panic was reported, there were only native processes which are > >> non-killable as OOM_SCORE_ADJ_MIN. > >> > >> After looking into the dump, I've found the dma-buf system heap was > >> trying to allocate a huge size. It seems to be a signed negative value= . > >> > >> dma_heap_ioctl_allocate(inline) > >> | heap_allocation =3D 0xFFFFFFC02247BD38 -> ( > >> | len =3D 0xFFFFFFFFE7225100, > >> > >> Actually the old ion system heap had policy which does not allow that > >> huge size with commit c9e8440eca61 ("staging: ion: Fix overflow and li= st > >> bugs in system heap"). We need this change again. Single allocation > >> should not be bigger than half of all memory. > >> > >> Signed-off-by: Jaewon Kim > >> --- > >> drivers/dma-buf/heaps/system_heap.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/hea= ps/system_heap.c > >> index e8bd10e60998..4c1ef2ecfb0f 100644 > >> --- a/drivers/dma-buf/heaps/system_heap.c > >> +++ b/drivers/dma-buf/heaps/system_heap.c > >> @@ -351,6 +351,9 @@ static struct dma_buf *system_heap_allocate(struct= dma_heap *heap, > >> struct page *page, *tmp_page; > >> int i, ret =3D -ENOMEM; > >> > >> + if (len / PAGE_SIZE > totalram_pages() / 2) > >> + return ERR_PTR(-ENOMEM); > >> + > > > >Instead of policy like that, would __GFP_RETRY_MAYFAIL on the system > >heap's LOW_ORDER_GFP flags also avoid the panic, and eventually fail > >the allocation request? > > Hello T.J. > > Thank you for your opinion. > The __GFP_RETRY_MAYFAIL on LOW_ORDER_GFP seems to work. > > page allocation failure: order:0, mode:0x144dc2(GFP_HIGHUSER|__GFP_RETRY_= MAYFAIL|__GFP_COMP|__GFP_ZERO) > Node 0 active_anon:120kB inactive_anon:43012kB active_file:36kB inactive_= file:788kB > > I tried to test it, and the allocation stopped at very low file cache sit= uation without OoM panic > as we expected. The phone device was freezing for few seconds though. > > We can avoid OoM panic through either totalram_pages() / 2 check or __GFP= _RETRY_MAYFAIL. > > But I think we still need the totalram_pages() / 2 check so that we don't= have to suffer > the freezing in UX perspective. We may kill some critical processes or us= ers' recent apps. > > Regarding __GFP_RETRY_MAYFAIL, I think it will help us avoid OoM panic. B= ut I'm worried > about low memory devices which still need OoM kill to get memory like in = camera scenarios. > > So what do you think? > Hey Jaewon, thanks for checking! The totalram_pages() / 2 just feels somewhat arbitrary. On the lowest memory devices I'm aware of that use the system heap it would take a single buffer on the order of several hundred megabytes to exceed that, so I guess the simple check is fine here until someone says they just can't live without a buffer that big! Reviewed-by: T.J. Mercier