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 08369C433EF for ; Wed, 22 Jun 2022 12:08:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EB09D11333F; Wed, 22 Jun 2022 12:08:33 +0000 (UTC) Received: from madras.collabora.co.uk (madras.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3807F11333F for ; Wed, 22 Jun 2022 12:08:33 +0000 (UTC) Received: from maud (unknown [184.175.41.107]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: alyssa) by madras.collabora.co.uk (Postfix) with ESMTPSA id 72BA96601792; Wed, 22 Jun 2022 13:08:29 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1655899712; bh=OPrmD/1mhTg3+/bTF0sZDhaNhdN3Dc9Cr3c2DYFgrwo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=i8G7OZIsNtbeKDQ/prV+bbMa2GwrFz/9EF1gGaOKJb5g86k2uf32g6DVJMa1vradM vUzYMa177UI7Q9pRzdA43cXjgpYFoK0r/8cA/nif+qi97uAMB3JcIklGZkY3tu3zMQ 2L8BtnO0g3NFb5hD10yNBVz66J7Uggm8m7yN7lkl79v4pfdZia4eOuUGsF7z8NKWJg 9osoz73oC2wtaPKz24M4KXo7TiSd1zqCJJYlAxvfoNw1BW0n6fwc5lXVPwaFRtKfDM tfuLMW70tnsS0Wu0kcnnJ/DZkw22wZRP91lowv/AVzrMzlj+BWFq5vYKBUouWKG9mX n6RjVyJdcFR5Q== Date: Wed, 22 Jun 2022 08:08:24 -0400 From: Alyssa Rosenzweig To: Adri??n Larumbe Subject: Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump Message-ID: References: <20220621023204.94179-1-adrian.larumbe@collabora.com> <20220621023204.94179-2-adrian.larumbe@collabora.com> <20220622015455.pqiiqevouh7udj7y@sobremesa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220622015455.pqiiqevouh7udj7y@sobremesa> 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: dri-devel@lists.freedesktop.org, alyssa.rosenzweig@collabora.com, tomeu.vizoso@collabora.com, steven.price@arm.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" > > > + iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN | > > > + __GFP_NORETRY); > > > + if (!iter.start) { > > > + dev_warn(pfdev->dev, "failed to allocate devcoredump file\n"); > > > + return; > > > + } > > > ... > > > + memset(iter.hdr, 0, iter.data - iter.start); > > > > Why are we using __GFP_NOWARN and __GFP_NORETRY? Why not plain vmalloc? > > > > Also, why vmalloc instead of vzalloc? (Or adding __GFP_ZERO to the list > > of __vmalloc flags if __GFP_NOWARN/__GFP_NORETRY are really needed?) Are > > there relevant performance or security considerations? > > I borrowed this code from Etnaviv a while ago and the same doubt struck me > then. My understanding of its intended behaviour is that because the dump file > might be huge, we don't want the memory manager to trigger the OOM killer and > annoy quite a few running processes because of a debug feature. Also since the > code already handles the situation when an allocation fails by refusing to > generate a dump, there's no need for the allocator to generate specific error > messages. > > So I guess it boils down to 'if there's quite enough memory to allocate a huge > dump file, go ahead, otherwise don't reclaim any processes' pages for something > that isn't essential'. > > I don't see much use for __GFP_ZERO in this case, because the dump file gets > memcpy'd with the contents of every single bo so whatever the original > contents of the memory were at the time of the allocation, they're overwritten > immediately. I think that's a reasonable explanation, bearing in mind I'm firmly a userspace person ;-) Please add a comment explaining the assumptions here, though, because the code will live longer than this ML thread. > I've also rebased v3 on top of drm-misc-next and the compiler error because of > the removed panfrost_job structure member is gone. Excellent