[PATCH] fastboot: scale default sparse max_download_size with available memory

[PATCH] fastboot: scale default sparse max_download_size with available memory

[Ahmad Fatoum]

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>
---
 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;
-- 
2.39.2

[PATCH] fixup! fastboot: scale default sparse max_download_size with available memory

[Ahmad Fatoum]

mem_malloc_end is the end address, so we need to add one to get the size
out.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Forgot to squash
---
 include/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/memory.h b/include/memory.h
index 73642dd5aaa5..9c2a037610b6 100644
--- a/include/memory.h
+++ b/include/memory.h
@@ -12,7 +12,7 @@ ulong mem_malloc_end(void);
 
 static inline ulong mem_malloc_size(void)
 {
-	return mem_malloc_end() - mem_malloc_start();
+	return mem_malloc_end() - mem_malloc_start() + 1;
 }
 
 struct memory_bank {
-- 
2.39.2

Re: [PATCH] fixup! fastboot: scale default sparse max_download_size with available memory

[Sascha Hauer]

On Fri, Aug 25, 2023 at 07:29:39PM +0200, Ahmad Fatoum wrote:
> mem_malloc_end is the end address, so we need to add one to get the size
> out.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Forgot to squash
> ---
>  include/memory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Could you resend the whole patch instead of sending a fixup?

This is one of the cases where b4 only recognizes the fixup patch mail
as a patch, but not the one with the original patch:

Looking up https://lore.barebox.org/20230825172250.2857448-1-a.fatoum@pengutronix.de
Grabbing thread from lore.barebox.org/barebox/20230825172250.2857448-1-a.fatoum@pengutronix.de/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Nothing matching that query.
Analyzing 3 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH] fixup! fastboot: scale default sparse max_download_size with available memory
  ---
  ✓ Signed: DKIM/lists.infradead.org (From: a.fatoum@pengutronix.de)
---
Total patches: 1
---
Applying: fixup! fastboot: scale default sparse max_download_size with available memory
Patch failed at 0001 fixup! fastboot: scale default sparse max_download_size with available memory
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: patch failed: include/memory.h:12
error: include/memory.h: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch

Sascha

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

Re: [PATCH] fastboot: scale default sparse max_download_size with available memory

[Johannes Zink]

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    |