Message ID | 0b4eeb86dda93a90a4581a0814d4b8c8f99707f3.1439895555.git.marcel.ziswiler@toradex.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Hi Marcel, On 18 August 2015 at 05:06, Marcel Ziswiler <marcel@ziswiler.com> wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer > alignment into account which led to failures of the following form: > > ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 > ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108 > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Acked-by: Scott Wood <scottwood@freescale.com> > --- > Changes in v6: fix assembler build breakage caused by including regular > header file like malloc.h by wrapping my wrapper in __ASSEMBLY__ > Changes in v5: split up into separate patches to be picked up by the > various subsystem maintainers as suggested by Marek > Changes in v4: move wrapper to common.h as suggested by Scott > Changes in v3: introduce malloc_cache_aligned() as suggested by Scott > Changes in v2: run it through checkpatch.pl and fix long lines > > common/cmd_ubi.c | 2 +- > drivers/mtd/nand/nand_util.c | 2 +- > fs/ubifs/super.c | 5 +++-- > fs/ubifs/ubifs.c | 4 ++-- > include/common.h | 9 +++++++++ > lib/gzip.c | 2 +- > 6 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c > index cbc10c5..10eea65 100644 > --- a/common/cmd_ubi.c > +++ b/common/cmd_ubi.c > @@ -363,7 +363,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) > tbuf_size = vol->usable_leb_size; > if (size < tbuf_size) > tbuf_size = ALIGN(size, ubi->min_io_size); > - tbuf = malloc(tbuf_size); > + tbuf = malloc_cache_aligned(tbuf_size); > if (!tbuf) { > printf("NO MEM\n"); > return ENOMEM; > diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c > index ee2c24d..21b4a61 100644 > --- a/drivers/mtd/nand/nand_util.c > +++ b/drivers/mtd/nand/nand_util.c > @@ -839,7 +839,7 @@ int nand_torture(nand_info_t *nand, loff_t offset) > > patt_count = ARRAY_SIZE(patterns); > > - buf = malloc(nand->erasesize); > + buf = malloc_cache_aligned(nand->erasesize); > if (buf == NULL) { > puts("Out of memory for erase block buffer\n"); > return -ENOMEM; > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index 10f8fff..0bf52db 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -57,7 +57,8 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino) > { > struct inode *inode; > > - inode = (struct inode *)malloc(sizeof(struct ubifs_inode)); > + inode = (struct inode *)malloc_cache_aligned( > + sizeof(struct ubifs_inode)); > if (inode) { > inode->i_ino = ino; > inode->i_sb = sb; > @@ -104,7 +105,7 @@ void iput(struct inode *inode) > /* > * Allocate and use new inode > */ > - ino = (struct inode *)malloc(sizeof(struct ubifs_inode)); > + ino = (struct inode *)malloc_cache_aligned(sizeof(struct ubifs_inode)); > memcpy(ino, inode, sizeof(struct ubifs_inode)); > > /* > diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c > index 6dd6174..4daa7fa 100644 > --- a/fs/ubifs/ubifs.c > +++ b/fs/ubifs/ubifs.c > @@ -108,7 +108,7 @@ static inline struct crypto_comp *crypto_alloc_comp(const char *alg_name, > struct crypto_comp *ptr; > int i = 0; > > - ptr = malloc(sizeof(struct crypto_comp)); > + ptr = malloc_cache_aligned(sizeof(struct crypto_comp)); > while (i < UBIFS_COMPR_TYPES_CNT) { > comp = ubifs_compressors[i]; > if (!comp) { > @@ -723,7 +723,7 @@ static int do_readpage(struct ubifs_info *c, struct inode *inode, > * destination area to a multiple of > * UBIFS_BLOCK_SIZE. > */ > - buff = malloc(UBIFS_BLOCK_SIZE); > + buff = malloc_cache_aligned(UBIFS_BLOCK_SIZE); > if (!buff) { > printf("%s: Error, malloc fails!\n", > __func__); > diff --git a/include/common.h b/include/common.h > index c48e5bc..c12f402 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * const argv[]); > #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size) \ > DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN) > > +#ifndef __ASSEMBLY__ > +#include <malloc.h> > + > +static inline void *malloc_cache_aligned(size_t size) > +{ > + return memalign(ARCH_DMA_MINALIGN, ALIGN(size, ARCH_DMA_MINALIGN)); > +} > +#endif Should this go in malloc.h? > + > /* > * check_member() - Check the offset of a structure member > * > diff --git a/lib/gzip.c b/lib/gzip.c > index ff37d4f..cd8e9fe 100644 > --- a/lib/gzip.c > +++ b/lib/gzip.c > @@ -25,7 +25,7 @@ static void *zalloc(void *x, unsigned items, unsigned size) > size *= items; > size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1); > > - p = malloc (size); > + p = malloc_cache_aligned(size); > > return (p); > } > -- > 2.4.3 > Regards, Simon
On Tue, 2015-08-18 at 06:44 -0600, Simon Glass wrote: > > diff --git a/include/common.h b/include/common.h > > index c48e5bc..c12f402 100644 > > --- a/include/common.h > > +++ b/include/common.h > > @@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * > > const argv[]); > > #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size) > > \ > > DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN) > > > > +#ifndef __ASSEMBLY__ > > +#include <malloc.h> > > + > > +static inline void *malloc_cache_aligned(size_t size) > > +{ > > + return memalign(ARCH_DMA_MINALIGN, ALIGN(size, > > ARCH_DMA_MINALIGN)); > > +} > > +#endif > > Should this go in malloc.h? I had it there before but Scott suggested otherwise: https://patchwork.ozlabs.org/patch/503291/
On Tue, 2015-08-18 at 06:44 -0600, Simon Glass wrote: > Hi Marcel, > > On 18 August 2015 at 05:06, Marcel Ziswiler <marcel@ziswiler.com> wrote: > > > > diff --git a/include/common.h b/include/common.h > > index c48e5bc..c12f402 100644 > > --- a/include/common.h > > +++ b/include/common.h > > @@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * const > > argv[]); > > #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size) \ > > DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN) > > > > +#ifndef __ASSEMBLY__ > > +#include <malloc.h> > > + > > +static inline void *malloc_cache_aligned(size_t size) > > +{ > > + return memalign(ARCH_DMA_MINALIGN, ALIGN(size, > > ARCH_DMA_MINALIGN)); > > +} > > +#endif > > Should this go in malloc.h? I previously asked for it to not go into malloc.h, in order to not mix upstream dlmalloc stuff with U-Boot things. -Scott
Hi, On 18 August 2015 at 09:49, Scott Wood <scottwood@freescale.com> wrote: > On Tue, 2015-08-18 at 06:44 -0600, Simon Glass wrote: >> Hi Marcel, >> >> On 18 August 2015 at 05:06, Marcel Ziswiler <marcel@ziswiler.com> wrote: >> > >> > diff --git a/include/common.h b/include/common.h >> > index c48e5bc..c12f402 100644 >> > --- a/include/common.h >> > +++ b/include/common.h >> > @@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * const >> > argv[]); >> > #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size) \ >> > DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN) >> > >> > +#ifndef __ASSEMBLY__ >> > +#include <malloc.h> >> > + >> > +static inline void *malloc_cache_aligned(size_t size) >> > +{ >> > + return memalign(ARCH_DMA_MINALIGN, ALIGN(size, >> > ARCH_DMA_MINALIGN)); >> > +} >> > +#endif >> >> Should this go in malloc.h? > > I previously asked for it to not go into malloc.h, in order to not mix > upstream dlmalloc stuff with U-Boot things. That's fine. It's a bit of a mess in there. Regards, Simon
On Tue, Aug 18, 2015 at 01:06:37PM +0200, Marcel Ziswiler wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer > alignment into account which led to failures of the following form: > > ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 > ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108 > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Acked-by: Scott Wood <scottwood@freescale.com> Applied to u-boot/master, thanks!
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c index cbc10c5..10eea65 100644 --- a/common/cmd_ubi.c +++ b/common/cmd_ubi.c @@ -363,7 +363,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) tbuf_size = vol->usable_leb_size; if (size < tbuf_size) tbuf_size = ALIGN(size, ubi->min_io_size); - tbuf = malloc(tbuf_size); + tbuf = malloc_cache_aligned(tbuf_size); if (!tbuf) { printf("NO MEM\n"); return ENOMEM; diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index ee2c24d..21b4a61 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -839,7 +839,7 @@ int nand_torture(nand_info_t *nand, loff_t offset) patt_count = ARRAY_SIZE(patterns); - buf = malloc(nand->erasesize); + buf = malloc_cache_aligned(nand->erasesize); if (buf == NULL) { puts("Out of memory for erase block buffer\n"); return -ENOMEM; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 10f8fff..0bf52db 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -57,7 +57,8 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino) { struct inode *inode; - inode = (struct inode *)malloc(sizeof(struct ubifs_inode)); + inode = (struct inode *)malloc_cache_aligned( + sizeof(struct ubifs_inode)); if (inode) { inode->i_ino = ino; inode->i_sb = sb; @@ -104,7 +105,7 @@ void iput(struct inode *inode) /* * Allocate and use new inode */ - ino = (struct inode *)malloc(sizeof(struct ubifs_inode)); + ino = (struct inode *)malloc_cache_aligned(sizeof(struct ubifs_inode)); memcpy(ino, inode, sizeof(struct ubifs_inode)); /* diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 6dd6174..4daa7fa 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -108,7 +108,7 @@ static inline struct crypto_comp *crypto_alloc_comp(const char *alg_name, struct crypto_comp *ptr; int i = 0; - ptr = malloc(sizeof(struct crypto_comp)); + ptr = malloc_cache_aligned(sizeof(struct crypto_comp)); while (i < UBIFS_COMPR_TYPES_CNT) { comp = ubifs_compressors[i]; if (!comp) { @@ -723,7 +723,7 @@ static int do_readpage(struct ubifs_info *c, struct inode *inode, * destination area to a multiple of * UBIFS_BLOCK_SIZE. */ - buff = malloc(UBIFS_BLOCK_SIZE); + buff = malloc_cache_aligned(UBIFS_BLOCK_SIZE); if (!buff) { printf("%s: Error, malloc fails!\n", __func__); diff --git a/include/common.h b/include/common.h index c48e5bc..c12f402 100644 --- a/include/common.h +++ b/include/common.h @@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * const argv[]); #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size) \ DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN) +#ifndef __ASSEMBLY__ +#include <malloc.h> + +static inline void *malloc_cache_aligned(size_t size) +{ + return memalign(ARCH_DMA_MINALIGN, ALIGN(size, ARCH_DMA_MINALIGN)); +} +#endif + /* * check_member() - Check the offset of a structure member * diff --git a/lib/gzip.c b/lib/gzip.c index ff37d4f..cd8e9fe 100644 --- a/lib/gzip.c +++ b/lib/gzip.c @@ -25,7 +25,7 @@ static void *zalloc(void *x, unsigned items, unsigned size) size *= items; size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1); - p = malloc (size); + p = malloc_cache_aligned(size); return (p); }