mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Johannes Zink <j.zink@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>, barebox@lists.infradead.org
Cc: rhi@pengutronix.de, jzi@pengutronix.de, ejo@pengutronix.de
Subject: Re: [PATCH] fastboot: scale default sparse max_download_size with available memory
Date: Mon, 28 Aug 2023 08:19:48 +0200	[thread overview]
Message-ID: <b3093699-65ac-1860-513d-02232c8fb873@pengutronix.de> (raw)
In-Reply-To: <20230825172250.2857448-1-a.fatoum@pengutronix.de>

On 8/25/23 19:22, Ahmad Fatoum wrote:
> The fastboot host tool chokes when asked to sparse images that are not
> 4K-aligned:
> 
>    error: write_sparse_skip_chunk: don't care size 4280912 is not a multiple
>    of the block size 4096
> 
> This is often not a problem, because disk images tend to be 4K-aligned
> and are best converted to sparse images properly explicitly anyway, so
> don't care holes are preserved.
> 
> Bootloader binaries on the other hand are smaller than the default
> fastboot.max_download_size of 8M, so they are downloaded as a single
> chunk without problems.
> 
> Situation is different for kernel and FIT images, which aren't
> necessarily 4K aligned, usually bigger than 8M, and are usually
> compressed, so explicitly sparsing them doesn't save time and thus is
> usually not done.
> 
> Short of fixing the fastboot host tool, users can skip sparsing kernel
> images as well by choosing a suitable larger fastboot.max_download_size
> than the 8M default.
> 
> To improve user experience, let's scale max_download_size with the
> available memory. The minimum default value remains 8M, but the value
> can go up to 128M now and is computed by dividing available malloc
> area by 8 (roughly 1/16th of total SDRAM). For ARM systems, buffer
> sizes would now look like this:
> 
>    SDRAM Size	Fastboot Buffer size
> 
>    1024M		64M
>     512M		32M
>     256M		16M
>     128M		8M
> 
> To err on the side of caution, we only generate powers-of-two sizes.
> This only affects the default value and users can as before override
> it in the environment.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

LGTM

Reviewed-by: Johannes Zink <j.zink@pengutronix.de>

> ---
>   common/fastboot.c | 9 ++++++++-
>   include/memory.h  | 5 +++++
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/common/fastboot.c b/common/fastboot.c
> index f91f398d7a95..645aaa6a9f3f 100644
> --- a/common/fastboot.c
> +++ b/common/fastboot.c
> @@ -30,7 +30,9 @@
>   #include <ubiformat.h>
>   #include <unistd.h>
>   #include <magicvar.h>
> +#include <linux/log2.h>
>   #include <linux/sizes.h>
> +#include <memory.h>
>   #include <progress.h>
>   #include <environment.h>
>   #include <globalvar.h>
> @@ -940,9 +942,14 @@ struct file_list *get_fastboot_partitions(void)
>   
>   static int fastboot_globalvars_init(void)
>   {
> -	if (IS_ENABLED(CONFIG_FASTBOOT_SPARSE))
> +	if (IS_ENABLED(CONFIG_FASTBOOT_SPARSE)) {
> +		fastboot_max_download_size
> +			= roundup_pow_of_two(clamp_t(ulong, mem_malloc_size() / 8,
> +						     SZ_8M, SZ_128M));
>   		globalvar_add_simple_int("fastboot.max_download_size",
>   				 &fastboot_max_download_size, "%u");
> +	}
> +
>   	globalvar_add_simple_bool("fastboot.bbu", &fastboot_bbu);
>   	globalvar_add_simple_string("fastboot.partitions",
>   				    &fastboot_partitions);
> diff --git a/include/memory.h b/include/memory.h
> index ffd66db02ba0..73642dd5aaa5 100644
> --- a/include/memory.h
> +++ b/include/memory.h
> @@ -10,6 +10,11 @@ void mem_malloc_init(void *start, void *end);
>   ulong mem_malloc_start(void);
>   ulong mem_malloc_end(void);
>   
> +static inline ulong mem_malloc_size(void)
> +{
> +	return mem_malloc_end() - mem_malloc_start();
> +}
> +
>   struct memory_bank {
>   	struct list_head list;
>   	unsigned long start;

-- 
Pengutronix e.K.                | Johannes Zink                  |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |




      parent reply	other threads:[~2023-08-28  6:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-25 17:22 Ahmad Fatoum
2023-08-25 17:29 ` [PATCH] fixup! " Ahmad Fatoum
2023-08-28  7:57   ` Sascha Hauer
2023-08-28  6:19 ` Johannes Zink [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b3093699-65ac-1860-513d-02232c8fb873@pengutronix.de \
    --to=j.zink@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=ejo@pengutronix.de \
    --cc=jzi@pengutronix.de \
    --cc=rhi@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox