[U-Boot] [PATCH 1/2] fs: fat: dynamically allocate memory for temporary buffer

[Marek Vasut]

On 1/17/19 7:52 AM, tien.fong.chee at intel.com wrote:
> From: Stefan Agner <stefan.ag... at toradex.com>
> 
> Drop the statically allocated get_contents_vfatname_block and
> dynamically allocate a buffer only if required. This saves
> 64KiB of memory.
> 
> Signed-off-by: Stefan Agner <stefan.ag... at toradex.com>
> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> ---
>  fs/fat/fat.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 8803fb4..aa4636d 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -307,9 +307,6 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>   * into 'buffer'.
>   * Update the number of bytes read in *gotsize or return -1 on fatal errors.
>   */
> -__u8 get_contents_vfatname_block[MAX_CLUSTSIZE]
> -	__aligned(ARCH_DMA_MINALIGN);
> -
>  static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  			__u8 *buffer, loff_t maxsize, loff_t *gotsize)
>  {
> @@ -320,7 +317,7 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  	loff_t actsize;
>  
>  	*gotsize = 0;
> -	debug("Filesize: %llu bytes\n", filesize);
> +	debug("Filesize: %llu bytes, starting at pos %llu\n", filesize, pos);

This looks like a separate change.

>  	if (pos >= filesize) {
>  		debug("Read position past EOF: %llu\n", pos);
> @@ -352,15 +349,25 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  
>  	/* align to beginning of next cluster if any */
>  	if (pos) {
> +		__u8 *tmp_buffer;
> +
> +		tmp_buffer = malloc_cache_aligned(MAX_CLUSTSIZE);

Don't you know the cluster size by now ?

> +		if (!tmp_buffer) {
> +			debug("Error: allocating buffer\n");
> +			return -ENOMEM;
> +		}
> +
>  		actsize = min(filesize, (loff_t)bytesperclust);
> -		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
> +		if (get_cluster(mydata, curclust, tmp_buffer,
>  				(int)actsize) != 0) {
>  			printf("Error reading cluster\n");
> +			free(tmp_buffer);
>  			return -1;
>  		}
>  		filesize -= actsize;
>  		actsize -= pos;
> -		memcpy(buffer, get_contents_vfatname_block + pos, actsize);
> +		memcpy(buffer, tmp_buffer + pos, actsize);
> +		free(tmp_buffer);

How many times is this malloc()/free() called ? I wonder how much this
slows things down and how much it fragments the heap. Maybe the amount
of calls to this can be reduced somehow.

>  		*gotsize += actsize;
>  		if (!filesize)
>  			return 0;
> 


-- 
Best regards,
Marek Vasut