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