From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Schwierzeck Date: Wed, 30 Sep 2020 23:40:07 +0200 Subject: [RFC PATCH v2 1/1] Fix missing __udivdi3 in SquashFS implementation. In-Reply-To: <20200930154511.630103-2-mc5686@mclink.it> References: <20200930154511.630103-1-mc5686@mclink.it> <20200930154511.630103-2-mc5686@mclink.it> Message-ID: <421a215dbe69fd9351b939707fe0a5b0fdce5e89.camel@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 > #include > #include > -#include > +#include > > #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