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

Marek Vasut
2019-01-18T06:26:40Z

On 1/17/19 7:52 AM, tien.fong.chee at intel.com wrote:
> From: Stefan Agner <stefan.ag...@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...@toradex.com>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@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