On Thu, May 12, 2022 at 08:41:15PM +0200, Pali Rohár wrote:
On Thursday 12 May 2022 20:02:33 Sean Nyekjaer wrote:
On Thu, May 12, 2022 at 12:24:01PM +0200, Pali Rohár wrote:
On Thursday 12 May 2022 12:19:01 Pali Rohár wrote:
On Monday 09 May 2022 15:08:15 Miquel Raynal wrote:
Hello,
trini@konsulko.com wrote on Thu, 5 May 2022 09:15:00 -0400:
On Thu, May 05, 2022 at 11:26:37AM +0200, Sean Nyekjaer wrote:
> From: Sean Nyekjaer sean.nyekjaer.ext@siemensgamesa.com > > When compling for x86: > ld.bfd: fs/squashfs/sqfs.o: in function `sqfs_read': > u-boot/fs/squashfs/sqfs.c:1443: undefined reference to `__udivmoddi4' > ld.bfd: u-boot/fs/squashfs/sqfs.c:1521: undefined reference to `__udivmoddi4' > > Signed-off-by: Sean Nyekjaer sean.nyekjaer.ext@siemensgamesa.com > --- > fs/squashfs/sqfs.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c > index 5d9c52af80..6405db4ff3 100644 > --- a/fs/squashfs/sqfs.c > +++ b/fs/squashfs/sqfs.c > @@ -8,6 +8,7 @@ > */ > > #include <asm/unaligned.h> > +#include <div64.h> > #include <errno.h> > #include <fs.h> > #include <linux/types.h> > @@ -1440,7 +1441,7 @@ 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 = do_div(data_offset, ctxt.cur_dev->blksz); > 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, > @@ -1516,7 +1517,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len, > goto out; > } > > - start = frag_entry.start / ctxt.cur_dev->blksz; > + start = do_div(frag_entry.start, ctxt.cur_dev->blksz); > 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);
Adding maintainers...
I guess that's a correct fix, so
Reviewed-by: Miquel Raynal miquel.raynal@bootlin.com
Thanks, Miquèl
Hello! do_div() macro modifies its first argument. It is correct? Because previous code did not modify anything.
- The semantics of do_div() are:
- u32 do_div(u64 *n, u32 base)
- {
- u32 remainder = *n % base;
- *n = *n / base;
- return remainder;
- }
And this patch replaced:
start = data_offset / ctxt.cur_dev->blksz;
by:
start = do_div(data_offset, ctxt.cur_dev->blksz);
which expands to:
start = data_offset % ctxt.cur_dev->blksz; data_offset = data_offset / ctxt.cur_dev->blksz;
Which is wrong as it calculates something totally different.
So this patch is incorrect.
Hi Pali,
Yes I agree, I got tricked by some wrong cache from Yocto :/ I have tried lldiv(), and that works on our x86 target.
Do you have an opinion on that? Or do you have another idea to avoid the "undefined reference to `__udivmoddi4'" error?
/Sean
Hello! You really should not use C division operator for 64-bit integers as gcc still is not able to generate assembler code for such division on many 32-bit platforms.
You should use some function or macro from linux/math64.h or div64.h header file. For every combination of types, pairs and required operation there is specific function/macro (e.g. u64/u32, u64%u32, u64/u64, etc...).
I think that lldiv() is the correct call here. This function expects u64 as dividend, u32 as divisor and returns u64 result.
Thank you for the explanation :) Updated patch should be on the list.
/Sean