[PATCH] fs/squashfs: use do_div function for math

[Michael Nazzareno Trimarchi]

Hi

Il gio 12 mag 2022, 20:02 Sean Nyekjaer <sean at geanix.com> ha scritto:

> 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 at 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 at 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 at 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 at 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.
>

start = data_offset;
do_div(start, ctxt.cur_dev->blksz);

Something like this?

Michael


> Do you have an opinion on that?
> Or do you have another idea to avoid the
> "undefined reference to `__udivmoddi4'" error?
>
> /Sean
>