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 7768FCA5500 for ; Wed, 13 Sep 2023 07:36:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235421AbjIMHgs (ORCPT ); Wed, 13 Sep 2023 03:36:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58236 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230004AbjIMHgq (ORCPT ); Wed, 13 Sep 2023 03:36:46 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ABC421727; Wed, 13 Sep 2023 00:36:42 -0700 (PDT) Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madras.collabora.co.uk (Postfix) with ESMTPSA id 29A3C6607319; Wed, 13 Sep 2023 08:36:40 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1694590600; bh=BoaxSmoEMOcQoMOW33cqNIU5YTH3LaR/kBpDcxa9Mtg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MVB4UZdGG1sGVKpv2lA3Ecoc1NWOzsc+C/PO55PphJsf94STiVCVsDY2R3D7RQuHX bB1H6jIjjFuswdr7pyL0clLnmEFe+HCxHyGxdbtcBYuGrqCXanA+N7bnCL+1AMcvkH LXL1X03aCVuheK5YjaOj9xx1p2NS8aQegZoWzih2qDRGLz47mz5EfZ0sPHq2cBtkAh Hj0uPC6QxDFcjug7ldcQR6GDLm91ruCD4xhSTgZyJvp2cZLraWmPrSSC+1LnuArx7Q QlJXWR5lg3+vw19v7UzSZLgsrHSSW3UsR3xf6h8rTjg4rwtDi+dNYuKFPTKJ6mdK7c RW+DbDu7jt9FA== Date: Wed, 13 Sep 2023 09:36:37 +0200 From: Boris Brezillon To: Rob Clark Cc: =?UTF-8?B?QWRyacOhbg==?= Larumbe , maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, quic_abhinavk@quicinc.com, dmitry.baryshkov@linaro.org, sean@poorly.run, marijn.suijten@somainline.org, robh@kernel.org, steven.price@arm.com, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, healych@amazon.com, kernel@collabora.com, freedreno@lists.freedesktop.org, Tvrtko Ursulin Subject: Re: [PATCH v4 6/6] drm/drm-file: Show finer-grained BO sizes in drm_show_memory_stats Message-ID: <20230913093637.2748d217@collabora.com> In-Reply-To: References: <20230912084044.955864-1-adrian.larumbe@collabora.com> <20230912084044.955864-7-adrian.larumbe@collabora.com> <20230912113210.65897aab@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 12 Sep 2023 19:14:35 -0700 Rob Clark wrote: > On Tue, Sep 12, 2023 at 6:46=E2=80=AFPM Rob Clark w= rote: > > > > On Tue, Sep 12, 2023 at 2:32=E2=80=AFAM Boris Brezillon > > wrote: =20 > > > > > > On Tue, 12 Sep 2023 09:37:00 +0100 > > > Adri=C3=A1n Larumbe wrote: > > > =20 > > > > The current implementation will try to pick the highest available s= ize > > > > display unit as soon as the BO size exceeds that of the previous > > > > multiplier. That can lead to loss of precision in BO's whose size is > > > > not a multiple of a MiB. > > > > > > > > Fix it by changing the unit selection criteria. > > > > > > > > For much bigger BO's, their size will naturally be aligned on somet= hing > > > > bigger than a 4 KiB page, so in practice it is very unlikely their = display > > > > unit would default to KiB. =20 > > > > > > Let's wait for Rob's opinion on this. =20 > > > > This would mean that if you have SZ_1G + SZ_1K worth of buffers, you'd > > report the result in KiB.. which seems like overkill to me, esp given > > that the result is just a snapshot in time of a figure that > > realistically is dynamic. Yeah, my point was that, generally, such big buffers tend to have a bigger size alignment (like 2MB for anything bigger than 1GB), but maybe this assumption doesn't stand for all drivers. > > > > Maybe if you have SZ_1G+SZ_1K worth of buffers you should report the > > result with more precision than GiB, but more than MiB seems a bit > > overkill. > > > > BR, > > -R > > =20 > > > > > > > > Signed-off-by: Adri=C3=A1n Larumbe > > > > --- > > > > drivers/gpu/drm/drm_file.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > > > > index 762965e3d503..bf7d2fe46bfa 100644 > > > > --- a/drivers/gpu/drm/drm_file.c > > > > +++ b/drivers/gpu/drm/drm_file.c > > > > @@ -879,7 +879,7 @@ static void print_size(struct drm_printer *p, c= onst char *stat, > > > > unsigned u; > > > > > > > > for (u =3D 0; u < ARRAY_SIZE(units) - 1; u++) { > > > > - if (sz < SZ_1K) =20 >=20 > btw, I was thinking more along the lines of: >=20 > if (sz < 10*SZ_1K) >=20 > (or perhaps maybe 100*SZ_1K) I think I suggested doing that at some point: if ((sz & (SZ_1K - 1)) && sz < UPPER_UNIT_THRESHOLD * SZ_1K) break; so we can keep using the upper unit if the size is a multiple of this upper unit, even if it's smaller than the selected threshold. >=20 > I mean, any visualization tool is going to scale the y axis based on > the order of magnitude.. and if I'm looking at the fdinfo with my > eyeballs I don't want to count the # of digits manually to do the > conversion in my head. The difference btwn 4 or 5 or maybe 6 digits > is easy enough to eyeball, but more than that is too much for my > eyesight, and I'm not seeing how it is useful ;-) >=20 > But if someone really has a valid use case for having precision in 1KB > then I'm willing to be overruled. So, precision loss was one aspect, but my main concern was having things displayed in KiB when they could have been displayed in MiB, because the size is a multiple of a MiB but still not big enough to pass the threshold test (which was set to 10000x in the previous version). > But I'm not a fan of the earlier > approach of different drivers reporting results differently, the whole > point of fdinfo was to have some standardized reporting. Totally agree with that. >=20 > BR, > -R >=20 > > > > + if (sz & (SZ_1K - 1)) > > > > break; > > > > sz =3D div_u64(sz, SZ_1K); > > > > } =20 > > > =20