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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 533C8EB64D9 for ; Fri, 7 Jul 2023 12:28:50 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 4EDB2865DB; Fri, 7 Jul 2023 14:27:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=163.com header.i=@163.com header.b="RBnJzVbF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8146D863C0; Fri, 7 Jul 2023 12:54:28 +0200 (CEST) Received: from m12.mail.163.com (m12.mail.163.com [220.181.12.214]) by phobos.denx.de (Postfix) with ESMTP id 86AE4861E6 for ; Fri, 7 Jul 2023 12:54:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=qianfanguijin@163.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:From: Content-Type; bh=8nbGbSo6lcs6zfrtvlln4uNxlS4fT494OFnL16rNcDY=; b=RBnJzVbFqRfxW50RIfbIrK05yAN66cJkew9pjVcgBeu1PJpVuB7GHEf2s3lXDG Cs5FySZS5/qr4BOFyHp4R689vOY/npStNKUlDo2uooGMLaW1Etj5nrZNm+nkXrFR uvpsLDCQEOa5PFToIJkHxEkqzfxYJZXXDIorXGmfg66aI= Received: from [172.21.25.67] (unknown [218.201.129.19]) by zwqz-smtp-mta-g1-2 (Coremail) with SMTP id _____wCHiiLU7qdkWwiiBw--.56596S2; Fri, 07 Jul 2023 18:54:12 +0800 (CST) Message-ID: <0fb12072-776b-e67c-1a50-4f6272e8213f@163.com> Date: Fri, 7 Jul 2023 18:54:13 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2] lib: sparse: allocate FASTBOOT_MAX_BLK_WRITE instead of small number To: Mattijs Korpershoek , Sean Anderson Cc: Guillaume La Roque , Gary Bisson , Troy Kisky , u-boot@lists.denx.de References: <20230616-sparse-flash-fix-v2-1-25717e6cbb27@baylibre.com> Content-Language: en-US From: qianfan In-Reply-To: <20230616-sparse-flash-fix-v2-1-25717e6cbb27@baylibre.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID: _____wCHiiLU7qdkWwiiBw--.56596S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxJFWkXryfZw43Ary5Cr4fuFg_yoW5tw4fpa yDCa9YkrZrGr4rCa17Ww1qvF10qa13AFyUK3Z5Z34Sva13tFyDur1UKryrursrWrZ3J3Zr Jwnagr1Yk3Z8trJanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07U92-5UUUUU= X-Originating-IP: [218.201.129.19] X-CM-SenderInfo: htld0w5dqj3xxmlqqiywtou0bp/1tbiXRal7VWBqKpj+AAAsO X-Mailman-Approved-At: Fri, 07 Jul 2023 14:27:05 +0200 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean 在 2023/7/7 16:13, Mattijs Korpershoek 写道: > Commit 62649165cb02 ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned") > fixed cache alignment for systems with a D-CACHE. > > However it introduced some performance regressions [1] on system > flashing huge images, such as Android. > > On AM62x SK EVM, we also observe such performance penalty: > Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.954s] > Writing 'super' OKAY [ 75.926s] > Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.641s] > Writing 'super' OKAY [ 62.849s] > Finished. Total time: 182.474s > > The reason for this is that we use an arbitrary small buffer > (info->blksz * 100) for transferring. > > Fix it by using a bigger buffer (info->blksz * FASTBOOT_MAX_BLK_WRITE) > as suggested in the original's patch review [2]. > > With this patch, performance impact is mitigated: > Sending sparse 'super' 1/2 (768793 KB) OKAY [ 23.912s] > Writing 'super' OKAY [ 15.780s] > Sending sparse 'super' 2/2 (629819 KB) OKAY [ 19.581s] > Writing 'super' OKAY [ 17.192s] > Finished. Total time: 76.569s > > [1] https://lore.kernel.org/r/20221118121323.4009193-1-gary.bisson@boundarydevices.com > [2] https://lore.kernel.org/r/all/43e4c17c-4483-ec8e-f843-9b4c5569bd18@seco.com/ > > Fixes: 62649165cb02 ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned") > Signed-off-by: Mattijs Korpershoek > --- > Changes in v2: > - Use FASTBOOT_MAX_BLK_WRITE instead of blkcnt (Qianfan) > - Link to v1: https://lore.kernel.org/r/20230616-sparse-flash-fix-v1-1-6bafeacc567b@baylibre.com > --- > drivers/fastboot/fb_mmc.c | 2 -- > include/image-sparse.h | 2 ++ > lib/image-sparse.c | 3 ++- > 3 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c > index 9d25c402028a..060918e49109 100644 > --- a/drivers/fastboot/fb_mmc.c > +++ b/drivers/fastboot/fb_mmc.c > @@ -19,8 +19,6 @@ > #include > #include > > -#define FASTBOOT_MAX_BLK_WRITE 16384 > - > #define BOOT_PARTITION_NAME "boot" > > struct fb_mmc_sparse { > diff --git a/include/image-sparse.h b/include/image-sparse.h > index 0572dbd0a283..282a0b256498 100644 > --- a/include/image-sparse.h > +++ b/include/image-sparse.h > @@ -7,6 +7,8 @@ > #include > #include > > +#define FASTBOOT_MAX_BLK_WRITE 16384 Hi: Just a personal suggestion, define sometings like FASTBOOT_MAX_BLK_WRITE in image-sparse.c is very strange. Or maybe define a marco such as SPARSE_MAX_BLK_WRITE and set a default value to 16384, and leave some comments for why we choice this value. Thanks. > + > #define ROUNDUP(x, y) (((x) + ((y) - 1)) & ~((y) - 1)) > > struct sparse_storage { > diff --git a/lib/image-sparse.c b/lib/image-sparse.c > index 5ec0f94ab3eb..8f8a67e15804 100644 > --- a/lib/image-sparse.c > +++ b/lib/image-sparse.c > @@ -55,7 +55,8 @@ static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info, > void *data, > char *response) > { > - lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100; > + lbaint_t n = blkcnt, write_blks, blks = 0; > + lbaint_t aligned_buf_blks = FASTBOOT_MAX_BLK_WRITE; > uint32_t *aligned_buf = NULL; > > if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) { > > --- > base-commit: 923de765ee1a5b26310f02cb42dcbad9e2b011c5 > change-id: 20230616-sparse-flash-fix-9c2852aa8d16 > > Best regards,