From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Thu, 8 Oct 2020 09:34:43 +0200
Subject: [fs/squashfs PATCH v3 2/2] avoid 64-bit divisions on 32-bit
In-Reply-To: <20201007223021.2443619-3-mc5686@mclink.it>
References: <20201007223021.2443619-1-mc5686@mclink.it>
 <20201007223021.2443619-3-mc5686@mclink.it>
Message-ID: <20201008093443.16feca42@xps13>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Mauro,

Mauro Condarelli <mc5686@mclink.it> wrote on Thu,  8 Oct 2020 00:30:21
+0200:

> Use macros in linux/kernel.h to avoid 64-bit divisions.

s/in/from/?

> 
> 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.
> 
> Some 32bit architectures (notably mipsel) lack an implementation of
> __udivdi3() compiler helper function in reduced libgcc.
> 
> Standard strategy is to use macros and do_div() inline.

You don't use do_div(), here, is it a leftover or do you mean that this
is a generic solution?

> 
> Signed-off-by: Mauro Condarelli <mc5686@mclink.it>
> ---
> 
> Changes in v3:
> - converted to use DIV_ROUND_(UP|DOWN)_ULL() macros (Miquel Raynal).
> - split commits to handle unrelated Kconfig warning (Thomas Petazzoni).
> 
> 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/sqfs.c       | 32 ++++++++++++++++----------------
>  fs/squashfs/sqfs_inode.c |  7 ++++---
>  2 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 15208b4dab..ef9f5e3449 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -10,14 +10,14 @@
>  #include <asm/unaligned.h>
>  #include <errno.h>
>  #include <fs.h>
> -#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <div64.h>
>  #include <linux/byteorder/little_endian.h>
>  #include <linux/byteorder/generic.h>
>  #include <memalign.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <squashfs.h>
> -#include <part.h>
>  
>  #include "sqfs_decompressor.h"
>  #include "sqfs_filesystem.h"
> @@ -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;
> +	start_ = DIV_ROUND_DOWN_ULL(le64_to_cpu(start), ctxt.cur_dev->blksz);
>  	*offset = le64_to_cpu(start) - (start_ * ctxt.cur_dev->blksz);
>  
> -	return DIV_ROUND_UP(table_size + *offset, ctxt.cur_dev->blksz);
> +	return (table_size + *offset + ctxt.cur_dev->blksz - 1) >> ctxt.cur_dev->log2blksz;

I don't recall Joao using this kind of structure but I might be wrong.
Can you just verify that this is not a leftover from a previous change?

Also, as you state in the commit message, all these divisions serve the
same purpose: translating a file length to a block number. I think a
helper would be very nice here, something like

	sqfs_length_to_block_id(ctxt, length);

Thanks,
Miqu?l