| Message ID | 1417030143-18083-1-git-send-email-trini@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Tom, On 26 November 2014 at 12:29, Tom Rini <trini@ti.com> wrote: > The changes to introduce loff_t into filesize means that we need to do > 64bit math on 32bit platforms. Make sure we use the right wrappers for > these operations. > > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com> > Cc: Suriyan Ramasami <suriyan.r@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > Signed-off-by: Tom Rini <trini@ti.com> This fixes things for me, thanks! But please see below. > --- > fs/ext4/ext4fs.c | 11 ++++++----- > fs/fat/fat_write.c | 10 ++++++---- > fs/fs.c | 6 ++++-- > 3 files changed, 16 insertions(+), 11 deletions(-) > > diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c > index 943b5bc..258b9379 100644 > --- a/fs/ext4/ext4fs.c > +++ b/fs/ext4/ext4fs.c > @@ -25,6 +25,7 @@ > #include <ext_common.h> > #include <ext4fs.h> > #include "ext4_common.h" > +#include <div64.h> > > int ext4fs_symlinknest; > struct ext_filesystem ext_fs; > @@ -67,11 +68,11 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > if (len > filesize) > len = filesize; > > - blockcnt = ((len + pos) + blocksize - 1) / blocksize; > + blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize); > > - for (i = pos / blocksize; i < blockcnt; i++) { > + for (i = lldiv(pos, blocksize); i < blockcnt; i++) { > lbaint_t blknr; > - int blockoff = pos % blocksize; > + int blockoff = pos - (blocksize * i); > int blockend = blocksize; > int skipfirst = 0; > blknr = read_allocated_block(&(node->inode), i); > @@ -82,7 +83,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > > /* Last block. */ > if (i == blockcnt - 1) { > - blockend = (len + pos) % blocksize; > + blockend = (len + pos) - (blocksize * i); Actually do you think it would be clearer to use div_u64_rem() here? > > /* The last portion is exactly blocksize. */ > if (!blockend) > @@ -90,7 +91,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > } > > /* First block. */ > - if (i == pos / blocksize) { > + if (i == lldiv(pos, blocksize)) { > skipfirst = blockoff; > blockend -= skipfirst; > } > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > index 88dd495..473723b 100644 > --- a/fs/fat/fat_write.c > +++ b/fs/fat/fat_write.c > @@ -13,6 +13,8 @@ > #include <asm/byteorder.h> > #include <part.h> > #include <linux/ctype.h> > +#include <div64.h> > +#include <linux/math64.h> > #include "fat.c" > > static void uppercase(char *str, int len) > @@ -770,7 +772,7 @@ static void fill_dentry(fsdata *mydata, dir_entry *dentptr, > */ > static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) > { > - __u32 startsect, sect_num; > + __u32 startsect, sect_num, offset; > > if (clustnum > 0) { > startsect = mydata->data_begin + > @@ -779,13 +781,13 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) > startsect = mydata->rootdir_sect; > } > > - sect_num = size / mydata->sect_size; > - if (size % mydata->sect_size) > + sect_num = div_u64_rem(size, mydata->sect_size, &offset); > + > + if (offset != 0) > sect_num++; > > if (startsect + sect_num > cur_part_info.start + total_sector) > return -1; > - > return 0; > } > > diff --git a/fs/fs.c b/fs/fs.c > index 3da7860..52078139 100644 > --- a/fs/fs.c > +++ b/fs/fs.c > @@ -23,6 +23,8 @@ > #include <fs.h> > #include <sandboxfs.h> > #include <asm/io.h> > +#include <div64.h> > +#include <linux/math64.h> > > DECLARE_GLOBAL_DATA_PTR; > > @@ -399,7 +401,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], > printf("%llu bytes read in %lu ms", len_read, time); > if (time > 0) { > puts(" ("); > - print_size(len_read / time * 1000, "/s"); > + print_size(div_u64(len_read, time * 1000), "/s"); The old code multiplies by 1000 but the new code divides. I think it should be: print_size(div_u64(len_read, time) * 1000, "/s"); > puts(")"); > } > puts("\n"); > @@ -469,7 +471,7 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], > printf("%llu bytes written in %lu ms", len, time); > if (time > 0) { > puts(" ("); > - print_size(len / time * 1000, "/s"); > + print_size(div_u64(len, time * 1000), "/s"); and here. > puts(")"); > } > puts("\n"); > -- > 1.7.9.5 > Regards, Simon
On Wed, Nov 26, 2014 at 12:36:27PM -0700, Simon Glass wrote: > Hi Tom, > > On 26 November 2014 at 12:29, Tom Rini <trini@ti.com> wrote: > > The changes to introduce loff_t into filesize means that we need to do > > 64bit math on 32bit platforms. Make sure we use the right wrappers for > > these operations. > > > > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com> > > Cc: Suriyan Ramasami <suriyan.r@gmail.com> > > Cc: Simon Glass <sjg@chromium.org> > > Signed-off-by: Tom Rini <trini@ti.com> > > This fixes things for me, thanks! But please see below. > > > --- > > fs/ext4/ext4fs.c | 11 ++++++----- > > fs/fat/fat_write.c | 10 ++++++---- > > fs/fs.c | 6 ++++-- > > 3 files changed, 16 insertions(+), 11 deletions(-) > > > > diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c > > index 943b5bc..258b9379 100644 > > --- a/fs/ext4/ext4fs.c > > +++ b/fs/ext4/ext4fs.c > > @@ -25,6 +25,7 @@ > > #include <ext_common.h> > > #include <ext4fs.h> > > #include "ext4_common.h" > > +#include <div64.h> > > > > int ext4fs_symlinknest; > > struct ext_filesystem ext_fs; > > @@ -67,11 +68,11 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > > if (len > filesize) > > len = filesize; > > > > - blockcnt = ((len + pos) + blocksize - 1) / blocksize; > > + blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize); > > > > - for (i = pos / blocksize; i < blockcnt; i++) { > > + for (i = lldiv(pos, blocksize); i < blockcnt; i++) { > > lbaint_t blknr; > > - int blockoff = pos % blocksize; > > + int blockoff = pos - (blocksize * i); > > int blockend = blocksize; > > int skipfirst = 0; > > blknr = read_allocated_block(&(node->inode), i); > > @@ -82,7 +83,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > > > > /* Last block. */ > > if (i == blockcnt - 1) { > > - blockend = (len + pos) % blocksize; > > + blockend = (len + pos) - (blocksize * i); > > Actually do you think it would be clearer to use div_u64_rem() here? I don't think so only since we don't care about the quotient only the remainder. In the FAT case we needed both (and that's what got me seeing the <linux/math64.h> functions. > > > > > /* The last portion is exactly blocksize. */ > > if (!blockend) > > @@ -90,7 +91,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, > > } > > > > /* First block. */ > > - if (i == pos / blocksize) { > > + if (i == lldiv(pos, blocksize)) { > > skipfirst = blockoff; > > blockend -= skipfirst; > > } > > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > > index 88dd495..473723b 100644 > > --- a/fs/fat/fat_write.c > > +++ b/fs/fat/fat_write.c > > @@ -13,6 +13,8 @@ > > #include <asm/byteorder.h> > > #include <part.h> > > #include <linux/ctype.h> > > +#include <div64.h> > > +#include <linux/math64.h> > > #include "fat.c" > > > > static void uppercase(char *str, int len) > > @@ -770,7 +772,7 @@ static void fill_dentry(fsdata *mydata, dir_entry *dentptr, > > */ > > static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) > > { > > - __u32 startsect, sect_num; > > + __u32 startsect, sect_num, offset; > > > > if (clustnum > 0) { > > startsect = mydata->data_begin + > > @@ -779,13 +781,13 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) > > startsect = mydata->rootdir_sect; > > } > > > > - sect_num = size / mydata->sect_size; > > - if (size % mydata->sect_size) > > + sect_num = div_u64_rem(size, mydata->sect_size, &offset); > > + > > + if (offset != 0) > > sect_num++; > > > > if (startsect + sect_num > cur_part_info.start + total_sector) > > return -1; > > - > > return 0; > > } > > > > diff --git a/fs/fs.c b/fs/fs.c > > index 3da7860..52078139 100644 > > --- a/fs/fs.c > > +++ b/fs/fs.c > > @@ -23,6 +23,8 @@ > > #include <fs.h> > > #include <sandboxfs.h> > > #include <asm/io.h> > > +#include <div64.h> > > +#include <linux/math64.h> > > > > DECLARE_GLOBAL_DATA_PTR; > > > > @@ -399,7 +401,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], > > printf("%llu bytes read in %lu ms", len_read, time); > > if (time > 0) { > > puts(" ("); > > - print_size(len_read / time * 1000, "/s"); > > + print_size(div_u64(len_read, time * 1000), "/s"); > > The old code multiplies by 1000 but the new code divides. I think it should be: > > print_size(div_u64(len_read, time) * 1000, "/s"); > > > puts(")"); > > } > > puts("\n"); > > @@ -469,7 +471,7 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], > > printf("%llu bytes written in %lu ms", len, time); > > if (time > 0) { > > puts(" ("); > > - print_size(len / time * 1000, "/s"); > > + print_size(div_u64(len, time * 1000), "/s"); > > and here. Whoops, those are bugs, fixed.
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 943b5bc..258b9379 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -25,6 +25,7 @@ #include <ext_common.h> #include <ext4fs.h> #include "ext4_common.h" +#include <div64.h> int ext4fs_symlinknest; struct ext_filesystem ext_fs; @@ -67,11 +68,11 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, if (len > filesize) len = filesize; - blockcnt = ((len + pos) + blocksize - 1) / blocksize; + blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize); - for (i = pos / blocksize; i < blockcnt; i++) { + for (i = lldiv(pos, blocksize); i < blockcnt; i++) { lbaint_t blknr; - int blockoff = pos % blocksize; + int blockoff = pos - (blocksize * i); int blockend = blocksize; int skipfirst = 0; blknr = read_allocated_block(&(node->inode), i); @@ -82,7 +83,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, /* Last block. */ if (i == blockcnt - 1) { - blockend = (len + pos) % blocksize; + blockend = (len + pos) - (blocksize * i); /* The last portion is exactly blocksize. */ if (!blockend) @@ -90,7 +91,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, } /* First block. */ - if (i == pos / blocksize) { + if (i == lldiv(pos, blocksize)) { skipfirst = blockoff; blockend -= skipfirst; } diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 88dd495..473723b 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -13,6 +13,8 @@ #include <asm/byteorder.h> #include <part.h> #include <linux/ctype.h> +#include <div64.h> +#include <linux/math64.h> #include "fat.c" static void uppercase(char *str, int len) @@ -770,7 +772,7 @@ static void fill_dentry(fsdata *mydata, dir_entry *dentptr, */ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) { - __u32 startsect, sect_num; + __u32 startsect, sect_num, offset; if (clustnum > 0) { startsect = mydata->data_begin + @@ -779,13 +781,13 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) startsect = mydata->rootdir_sect; } - sect_num = size / mydata->sect_size; - if (size % mydata->sect_size) + sect_num = div_u64_rem(size, mydata->sect_size, &offset); + + if (offset != 0) sect_num++; if (startsect + sect_num > cur_part_info.start + total_sector) return -1; - return 0; } diff --git a/fs/fs.c b/fs/fs.c index 3da7860..52078139 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -23,6 +23,8 @@ #include <fs.h> #include <sandboxfs.h> #include <asm/io.h> +#include <div64.h> +#include <linux/math64.h> DECLARE_GLOBAL_DATA_PTR; @@ -399,7 +401,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], printf("%llu bytes read in %lu ms", len_read, time); if (time > 0) { puts(" ("); - print_size(len_read / time * 1000, "/s"); + print_size(div_u64(len_read, time * 1000), "/s"); puts(")"); } puts("\n"); @@ -469,7 +471,7 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], printf("%llu bytes written in %lu ms", len, time); if (time > 0) { puts(" ("); - print_size(len / time * 1000, "/s"); + print_size(div_u64(len, time * 1000), "/s"); puts(")"); } puts("\n");
The changes to introduce loff_t into filesize means that we need to do 64bit math on 32bit platforms. Make sure we use the right wrappers for these operations. Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com> Cc: Suriyan Ramasami <suriyan.r@gmail.com> Cc: Simon Glass <sjg@chromium.org> Signed-off-by: Tom Rini <trini@ti.com> --- fs/ext4/ext4fs.c | 11 ++++++----- fs/fat/fat_write.c | 10 ++++++---- fs/fs.c | 6 ++++-- 3 files changed, 16 insertions(+), 11 deletions(-)