[PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"

[Sean Anderson]

On 11/18/22 07:13, Gary Bisson wrote:
> This reverts commit 62649165cb02ab95b57360bb362886935f524f26.
> 
> The patch decreased the write performance quite a bit.
> Here is an example on an i.MX 8M Quad platform.
> - Before the revert:
> Sending sparse 'vendor' 1/2 (516436 KB)            OKAY [  5.113s]
> Writing 'vendor'                                   OKAY [128.335s]
> Sending sparse 'vendor' 2/2 (76100 KB)             OKAY [  0.802s]
> Writing 'vendor'                                   OKAY [ 27.902s]
> - After the revert:
> Sending sparse 'vendor' 1/2 (516436 KB)            OKAY [  5.310s]
> Writing 'vendor'                                   OKAY [ 18.041s]
> Sending sparse 'vendor' 2/2 (76100 KB)             OKAY [  1.244s]
> Writing 'vendor'                                   OKAY [  2.663s]
> 
> Considering that the patch only moves buffer around to avoid a warning
> message about misaligned buffers, let's keep the best performances.

So what is the point of this warning?

--Sean

> Signed-off-by: Gary Bisson <gary.bisson at boundarydevices.com>
> Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
> ---
>  lib/image-sparse.c | 69 ++++++----------------------------------------
>  1 file changed, 8 insertions(+), 61 deletions(-)
> 
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index 5ec0f94ab3e..d80fdbbf58e 100644
> --- a/lib/image-sparse.c
> +++ b/lib/image-sparse.c
> @@ -46,66 +46,9 @@
>  #include <asm/cache.h>
>  
>  #include <linux/math64.h>
> -#include <linux/err.h>
>  
>  static void default_log(const char *ignored, char *response) {}
>  
> -static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
> -				       lbaint_t blk, lbaint_t blkcnt,
> -				       void *data,
> -				       char *response)
> -{
> -	lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
> -	uint32_t *aligned_buf = NULL;
> -
> -	if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
> -		write_blks = info->write(info, blk, n, data);
> -		if (write_blks < n)
> -			goto write_fail;
> -
> -		return write_blks;
> -	}
> -
> -	aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * aligned_buf_blks);
> -	if (!aligned_buf) {
> -		info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
> -		return -ENOMEM;
> -	}
> -
> -	while (blkcnt > 0) {
> -		n = min(aligned_buf_blks, blkcnt);
> -		memcpy(aligned_buf, data, n * info->blksz);
> -
> -		/* write_blks might be > n due to NAND bad-blocks */
> -		write_blks = info->write(info, blk + blks, n, aligned_buf);
> -		if (write_blks < n) {
> -			free(aligned_buf);
> -			goto write_fail;
> -		}
> -
> -		blks += write_blks;
> -		data += n * info->blksz;
> -		blkcnt -= n;
> -	}
> -
> -	free(aligned_buf);
> -	return blks;
> -
> -write_fail:
> -	if (IS_ERR_VALUE(write_blks)) {
> -		printf("%s: Write failed, block #" LBAFU " [" LBAFU "] (%lld)\n",
> -		       __func__, blk + blks, n, (long long)write_blks);
> -		info->mssg("flash write failure", response);
> -		return write_blks;
> -	}
> -
> -	/* write_blks < n */
> -	printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
> -	       __func__, blk + blks, n);
> -	info->mssg("flash write failure(incomplete)", response);
> -	return -1;
> -}
> -
>  int write_sparse_image(struct sparse_storage *info,
>  		       const char *part_name, void *data, char *response)
>  {
> @@ -209,11 +152,15 @@ int write_sparse_image(struct sparse_storage *info,
>  				return -1;
>  			}
>  
> -			blks = write_sparse_chunk_raw(info, blk, blkcnt,
> -						      data, response);
> -			if (blks < 0)
> +			blks = info->write(info, blk, blkcnt, data);
> +			/* blks might be > blkcnt (eg. NAND bad-blocks) */
> +			if (blks < blkcnt) {
> +				printf("%s: %s" LBAFU " [" LBAFU "]\n",
> +				       __func__, "Write failed, block #",
> +				       blk, blks);
> +				info->mssg("flash write failure", response);
>  				return -1;
> -
> +			}
>  			blk += blks;
>  			bytes_written += ((u64)blkcnt) * info->blksz;
>  			total_blocks += chunk_header->chunk_sz;