Am Mittwoch, den 30.09.2020, 17:45 +0200 schrieb Mauro Condarelli:
> Use right shift to avoid 64-bit divisions.
> 
> These divisions are needed to convert from file length (potentially
> over 32-bit range) to block number, so result and remainder are
> guaranteed to fit in 32-bit integers.
> 
> Signed-off-by: Mauro Condarelli <mc5...@mclink.it>

the commit subject needs to be fixed. If you change a specific
subsystem, you need to add the according prefix. You can use the Git
log to figure out that prefix. Also you don't fix a missing __udivdi3 ,
you just want to avoid 64-bit divisions.

So maybe something like: "fs/squashfs: avoid 64-bit divisions on 32-bit 
systems"

> ---
> 
> Changes in v2:
> - replace division with right shift (Daniel Schwierzeck).
> - remove vocore2-specific change (Daniel Schwierzeck).
> - add warning to Kconfig about CONFIG_SYS_MALLOC_LEN (Tom Rini).
> 
>  fs/squashfs/Kconfig      |  2 ++
>  fs/squashfs/sqfs.c       | 30 +++++++++++++++---------------
>  fs/squashfs/sqfs_inode.c |  6 +++---
>  3 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
> index 54ab1618f1..7c3f83d007 100644
> --- a/fs/squashfs/Kconfig
> +++ b/fs/squashfs/Kconfig
> @@ -9,3 +9,5 @@ config FS_SQUASHFS
>         filesystem use, for archival use (i.e. in cases where a .tar.gz file
>         may be used), and in constrained block device/memory systems (e.g.
>         embedded systems) where low overhead is needed.
> +       WARNING: if compression is enabled SquashFS needs a large amount
> +       of dynamic memory; make sure CONFIG_SYS_MALLOC_LEN >= 0x4000.
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 15208b4dab..90e6848e6c 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -85,10 +85,10 @@ static int sqfs_calc_n_blks(__le64 start, __le64 end, u64 
> *offset)
>       u64 start_, table_size;
>  
>       table_size = le64_to_cpu(end) - le64_to_cpu(start);
> -     start_ = le64_to_cpu(start) / ctxt.cur_dev->blksz;
> -     *offset = le64_to_cpu(start) - (start_ * ctxt.cur_dev->blksz);
> +     start_ = le64_to_cpu(start) >> ctxt.cur_dev->log2blksz;
> +     *offset = le64_to_cpu(start) - (start_ << ctxt.cur_dev->log2blksz);
>  
> -     return DIV_ROUND_UP(table_size + *offset, ctxt.cur_dev->blksz);
> +     return (table_size + *offset + ctxt.cur_dev->blksz - 1) >> 
> ctxt.cur_dev->log2blksz;
>  }
>  
>  /*
> @@ -109,8 +109,8 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>       if (inode_fragment_index >= get_unaligned_le32(&sblk->fragments))
>               return -EINVAL;
>  
> -     start = get_unaligned_le64(&sblk->fragment_table_start) /
> -             ctxt.cur_dev->blksz;
> +     start = get_unaligned_le64(&sblk->fragment_table_start) >>
> +             ctxt.cur_dev->log2blksz;
>       n_blks = sqfs_calc_n_blks(sblk->fragment_table_start,
>                                 sblk->export_table_start,
>                                 &table_offset);
> @@ -135,7 +135,7 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>       start_block = get_unaligned_le64(table + table_offset + block *
>                                        sizeof(u64));
>  
> -     start = start_block / ctxt.cur_dev->blksz;
> +     start = start_block >> ctxt.cur_dev->log2blksz;
>       n_blks = sqfs_calc_n_blks(cpu_to_le64(start_block),
>                                 sblk->fragment_table_start, &table_offset);
>  
> @@ -641,8 +641,8 @@ static int sqfs_read_inode_table(unsigned char 
> **inode_table)
>  
>       table_size = get_unaligned_le64(&sblk->directory_table_start) -
>               get_unaligned_le64(&sblk->inode_table_start);
> -     start = get_unaligned_le64(&sblk->inode_table_start) /
> -             ctxt.cur_dev->blksz;
> +     start = get_unaligned_le64(&sblk->inode_table_start) >>
> +             ctxt.cur_dev->log2blksz;
>       n_blks = sqfs_calc_n_blks(sblk->inode_table_start,
>                                 sblk->directory_table_start, &table_offset);
>  
> @@ -725,8 +725,8 @@ static int sqfs_read_directory_table(unsigned char 
> **dir_table, u32 **pos_list)
>       /* DIRECTORY TABLE */
>       table_size = get_unaligned_le64(&sblk->fragment_table_start) -
>               get_unaligned_le64(&sblk->directory_table_start);
> -     start = get_unaligned_le64(&sblk->directory_table_start) /
> -             ctxt.cur_dev->blksz;
> +     start = get_unaligned_le64(&sblk->directory_table_start) >>
> +             ctxt.cur_dev->log2blksz;
>       n_blks = sqfs_calc_n_blks(sblk->directory_table_start,
>                                 sblk->fragment_table_start, &table_offset);
>  
> @@ -1334,11 +1334,11 @@ int sqfs_read(const char *filename, void *buf, loff_t 
> offset, loff_t len,
>       }
>  
>       for (j = 0; j < datablk_count; j++) {
> -             start = data_offset / ctxt.cur_dev->blksz;
> +             start = data_offset >> ctxt.cur_dev->log2blksz;
>               table_size = SQFS_BLOCK_SIZE(finfo.blk_sizes[j]);
>               table_offset = data_offset - (start * ctxt.cur_dev->blksz);
> -             n_blks = DIV_ROUND_UP(table_size + table_offset,
> -                                   ctxt.cur_dev->blksz);
> +             n_blks = (table_size + table_offset + ctxt.cur_dev->blksz - 1) 
> >>
> +                     ctxt.cur_dev->log2blksz;
>  
>               data_buffer = malloc_cache_aligned(n_blks * 
> ctxt.cur_dev->blksz);
>  
> @@ -1388,10 +1388,10 @@ int sqfs_read(const char *filename, void *buf, loff_t 
> offset, loff_t len,
>               goto free_buffer;
>       }
>  
> -     start = frag_entry.start / ctxt.cur_dev->blksz;
> +     start = frag_entry.start >> ctxt.cur_dev->log2blksz;
>       table_size = SQFS_BLOCK_SIZE(frag_entry.size);
>       table_offset = frag_entry.start - (start * ctxt.cur_dev->blksz);
> -     n_blks = DIV_ROUND_UP(table_size + table_offset, ctxt.cur_dev->blksz);
> +     n_blks = (table_size + table_offset + ctxt.cur_dev->blksz - 1) >> 
> ctxt.cur_dev->log2blksz;
>  
>       fragment = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
>  
> diff --git a/fs/squashfs/sqfs_inode.c b/fs/squashfs/sqfs_inode.c
> index 1368f3063c..5dbf945c80 100644
> --- a/fs/squashfs/sqfs_inode.c
> +++ b/fs/squashfs/sqfs_inode.c
> @@ -10,7 +10,7 @@
>  #include <stdint.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <string.h>
> +#include <part.h>
>  
>  #include "sqfs_decompressor.h"
>  #include "sqfs_filesystem.h"
> @@ -68,9 +68,9 @@ int sqfs_inode_size(struct squashfs_base_inode *inode, u32 
> blk_size)
>               unsigned int blk_list_size;
>  
>               if (fragment == 0xFFFFFFFF)
> -                     blk_list_size = DIV_ROUND_UP(file_size, blk_size);
> +                     blk_list_size = (file_size + blk_size - 1) >> 
> LOG2(blk_size);
>               else
> -                     blk_list_size = file_size / blk_size;
> +                     blk_list_size = file_size > LOG2(blk_size);

one > is missing:

   blk_list_size = file_size >> LOG2(blk_size);

>  
>               return sizeof(*lreg) + blk_list_size * sizeof(u32);
>       }
-- 
- Daniel

Reply via email to