All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fs/squashfs: use do_div function for math
@ 2022-05-05  9:26 Sean Nyekjaer
  2022-05-05 13:15 ` Tom Rini
  0 siblings, 1 reply; 11+ messages in thread
From: Sean Nyekjaer @ 2022-05-05  9:26 UTC (permalink / raw)
  To: u-boot; +Cc: Sean Nyekjaer, trini

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);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-05  9:26 [PATCH] fs/squashfs: use do_div function for math Sean Nyekjaer
@ 2022-05-05 13:15 ` Tom Rini
  2022-05-09 13:08   ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Rini @ 2022-05-05 13:15 UTC (permalink / raw)
  To: Sean Nyekjaer, Joao Marcos Costa, Thomas Petazzoni, Miquel Raynal
  Cc: u-boot, Sean Nyekjaer

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

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

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-05 13:15 ` Tom Rini
@ 2022-05-09 13:08   ` Miquel Raynal
  2022-05-12 10:19     ` Pali Rohár
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2022-05-09 13:08 UTC (permalink / raw)
  To: Tom Rini
  Cc: Sean Nyekjaer, Joao Marcos Costa, Thomas Petazzoni, u-boot,
	Sean Nyekjaer

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-09 13:08   ` Miquel Raynal
@ 2022-05-12 10:19     ` Pali Rohár
  2022-05-12 10:24       ` Pali Rohár
  0 siblings, 1 reply; 11+ messages in thread
From: Pali Rohár @ 2022-05-12 10:19 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Tom Rini, Sean Nyekjaer, Joao Marcos Costa, Thomas Petazzoni,
	u-boot, Sean Nyekjaer

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 10:19     ` Pali Rohár
@ 2022-05-12 10:24       ` Pali Rohár
  2022-05-12 18:02         ` Sean Nyekjaer
  0 siblings, 1 reply; 11+ messages in thread
From: Pali Rohár @ 2022-05-12 10:24 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Tom Rini, Sean Nyekjaer, Joao Marcos Costa, Thomas Petazzoni,
	u-boot, Sean Nyekjaer

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 10:24       ` Pali Rohár
@ 2022-05-12 18:02         ` Sean Nyekjaer
  2022-05-12 18:23           ` Tom Rini
                             ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Sean Nyekjaer @ 2022-05-12 18:02 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Miquel Raynal, Tom Rini, Joao Marcos Costa, Thomas Petazzoni,
	u-boot, Sean Nyekjaer

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 18:02         ` Sean Nyekjaer
@ 2022-05-12 18:23           ` Tom Rini
  2022-05-12 18:41           ` Pali Rohár
  2022-05-12 18:52           ` Michael Nazzareno Trimarchi
  2 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2022-05-12 18:23 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: Pali Rohár, Miquel Raynal, Joao Marcos Costa,
	Thomas Petazzoni, u-boot, Sean Nyekjaer

[-- Attachment #1: Type: text/plain, Size: 3812 bytes --]

On Thu, May 12, 2022 at 08:02:33PM +0200, 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?

Please correct things to use lldiv(), thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 18:02         ` Sean Nyekjaer
  2022-05-12 18:23           ` Tom Rini
@ 2022-05-12 18:41           ` Pali Rohár
  2022-05-12 18:47             ` Sean Nyekjaer
  2022-05-12 18:52           ` Michael Nazzareno Trimarchi
  2 siblings, 1 reply; 11+ messages in thread
From: Pali Rohár @ 2022-05-12 18:41 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: Miquel Raynal, Tom Rini, Joao Marcos Costa, Thomas Petazzoni,
	u-boot, Sean Nyekjaer

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 18:41           ` Pali Rohár
@ 2022-05-12 18:47             ` Sean Nyekjaer
  0 siblings, 0 replies; 11+ messages in thread
From: Sean Nyekjaer @ 2022-05-12 18:47 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Miquel Raynal, Tom Rini, Joao Marcos Costa, Thomas Petazzoni,
	u-boot, Sean Nyekjaer

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 18:02         ` Sean Nyekjaer
  2022-05-12 18:23           ` Tom Rini
  2022-05-12 18:41           ` Pali Rohár
@ 2022-05-12 18:52           ` Michael Nazzareno Trimarchi
  2022-05-12 18:55             ` Pali Rohár
  2 siblings, 1 reply; 11+ messages in thread
From: Michael Nazzareno Trimarchi @ 2022-05-12 18:52 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: Pali Rohár, Miquel Raynal, Tom Rini, Joao Marcos Costa,
	Thomas Petazzoni, U-Boot-Denx, Sean Nyekjaer

Hi

Il gio 12 mag 2022, 20:02 Sean Nyekjaer <sean@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@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.
>

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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fs/squashfs: use do_div function for math
  2022-05-12 18:52           ` Michael Nazzareno Trimarchi
@ 2022-05-12 18:55             ` Pali Rohár
  0 siblings, 0 replies; 11+ messages in thread
From: Pali Rohár @ 2022-05-12 18:55 UTC (permalink / raw)
  To: Michael Nazzareno Trimarchi
  Cc: Sean Nyekjaer, Miquel Raynal, Tom Rini, Joao Marcos Costa,
	Thomas Petazzoni, U-Boot-Denx, Sean Nyekjaer

On Thursday 12 May 2022 20:52:30 Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Il gio 12 mag 2022, 20:02 Sean Nyekjaer <sean@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@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.
> >
> 
> start = data_offset;
> do_div(start, ctxt.cur_dev->blksz);
> 
> Something like this?

This is another option and seems that should be OK too.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-05-12 18:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05  9:26 [PATCH] fs/squashfs: use do_div function for math Sean Nyekjaer
2022-05-05 13:15 ` Tom Rini
2022-05-09 13:08   ` Miquel Raynal
2022-05-12 10:19     ` Pali Rohár
2022-05-12 10:24       ` Pali Rohár
2022-05-12 18:02         ` Sean Nyekjaer
2022-05-12 18:23           ` Tom Rini
2022-05-12 18:41           ` Pali Rohár
2022-05-12 18:47             ` Sean Nyekjaer
2022-05-12 18:52           ` Michael Nazzareno Trimarchi
2022-05-12 18:55             ` Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.