[PATCH 3/3] Fix missing __udivdi3 in SquashFS implementation.

Daniel Schwierzeck daniel.schwierzeck at gmail.com
Thu Sep 24 00:31:36 CEST 2020


Am Mittwoch, den 23.09.2020, 20:27 +0200 schrieb Mauro Condarelli:
> Thanks for the review,
> I'll prepare a v2 ASAP.
> 
> On 9/23/20 12:05 AM, Daniel Schwierzeck wrote:
> > Am Sonntag, den 20.09.2020, 21:21 -0400 schrieb Tom Rini:
> > > On Sun, Sep 20, 2020 at 06:29:01PM +0200, Mauro Condarelli wrote:
> > > 
> > > > Signed-off-by: Mauro Condarelli <mc5686 at mclink.it>
> > > > ---
> > > >  fs/squashfs/sqfs.c        | 45 +++++++++++++++++++++++++--------------
> > > >  fs/squashfs/sqfs_inode.c  |  8 +++----
> > > >  include/configs/vocore2.h |  2 +-
> > remove that file which is unrelated to this patch
> I will as this is fixing things just for my target and that is clearly wrong.
> OTOH I feel some provision should be implemented (probably at Config.in
> level) to ensure SquashFS has enough malloc space for its needs.
> What are the best practices to handle this?
> 
> 
> > > >  3 files changed, 34 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> > > > index 15208b4dab..b49331ce93 100644
> > > > --- a/fs/squashfs/sqfs.c
> > > > +++ b/fs/squashfs/sqfs.c
> > > > @@ -18,6 +18,8 @@
> > > >  #include <string.h>
> > > >  #include <squashfs.h>
> > > >  #include <part.h>
> > > > +#include <div64.h>
> > > > +#include <stdio.h>
> > > >  
> > > >  #include "sqfs_decompressor.h"
> > > >  #include "sqfs_filesystem.h"
> > > > @@ -82,13 +84,16 @@ static int sqfs_count_tokens(const char *filename)
> > > >   */
> > > >  static int sqfs_calc_n_blks(__le64 start, __le64 end, u64 *offset)
> > > >  {
> > > > -	u64 start_, table_size;
> > > > +	u64 start_, table_size, blks;
> > > >  
> > > >  	table_size = le64_to_cpu(end) - le64_to_cpu(start);
> > > > -	start_ = le64_to_cpu(start) / ctxt.cur_dev->blksz;
> > > > +	start_ = le64_to_cpu(start);
> > > > +	do_div(start_, ctxt.cur_dev->blksz);
> > have you tried with lldiv() which returns the 64bit result? Also it
> > would be a little cleaner:
> > 
> >     start_ = lldiv(le64_to_cpu(start), ctxt.cur_dev->blksz);
> I thought of that (actually my first attempt was quite similar,
> but I noticed that lldiv() actually uses do_div() internally and
> so I decided to go directly for the lower level (and presumably
> faster) solution.
> If You (or the maintainers) feel otherwise I can revert with
> no problems.

because do_div() replaces the dividend with the division remainder and
lldiv() returns the division result without modifying the dividend. 

Anyway I had a look at the Linux code and other U-Boot file systems and
there is a better solution. "struct blk_desc" has a member
"int log2blksz" which is initialized as "desc->log2blksz = LOG2(desc-
>blksz) in blk-uclass.c. With that you can replace all divisions with
"desc->blksz" as divisor with a simple right bit shift like so:

    start_ = le64_to_cpu(start) >> ctxt.cur_dev->log2blksz;

Likewise all multiplications can be replaced with a left bit shift,
e.g.:

    *offset = le64_to_cpu(start) - (start_ << ctxt.cur_dev->log2blksz);

Also the Linux code never uses DIV_ROUND_UP() for any block related
calculations. So maybe all DIV_ROUND_UP() calls can be replaced with a
bit shift too. 

-- 
- Daniel



More information about the U-Boot mailing list