Skip to contentSkip to conversation

mtd/nand/ubi: assortment of alignment fixes

PATCH RESEND Superseded Marcel Ziswiler 4 participants 6 messages Aug 18, 2015 – Aug 28, 2015

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

From: Marcel Ziswiler <[email protected]>

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

---
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(-)
common/cmd_ubi.c +1 -1
index cbc10c5..10eea65 100644
@@ -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;
drivers/mtd/nand/nand_util.c +1 -1
index ee2c24d..21b4a61 100644
@@ -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;
fs/ubifs/super.c +3 -2
index 10f8fff..0bf52db 100644
@@ -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));
 
 	/*
fs/ubifs/ubifs.c +2 -2
index 6dd6174..4daa7fa 100644
@@ -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__);
include/common.h +9 -0
index c48e5bc..c12f402 100644
@@ -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
  *
lib/gzip.c +1 -1
index ff37d4f..cd8e9fe 100644
@@ -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);
 }
Signature · 1 line
2.4.3
#1 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

Hi Marcel,

Quoted 105 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

---
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(-)
common/cmd_ubi.c +1 -1
index cbc10c5..10eea65 100644
@@ -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;
drivers/mtd/nand/nand_util.c +1 -1
index ee2c24d..21b4a61 100644
@@ -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;
fs/ubifs/super.c +3 -2
index 10f8fff..0bf52db 100644
@@ -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));
 
        /*
fs/ubifs/ubifs.c +2 -2
index 6dd6174..4daa7fa 100644
@@ -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__);
include/common.h +0 -0
index c48e5bc..c12f402 100644
@@ -1060,6 +1060,15 @@ int cpu_release(int nr, int argc, char * const argv[]);
 #define DEFINE_CACHE_ALIGN_BUFFER(type, name, size)                    \

include/common.h int cpu_release(int nr, int argc, char * const argv[]); · line 1070

Quoted 10 lines
include/common.h +8 -0
        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?

Quoted 18 lines
include/common.h +1 -0
+
 /*
  * check_member() - Check the offset of a structure member
  *
lib/gzip.c +1 -2
index ff37d4f..cd8e9fe 100644
@@ -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);
 }
--
Quoted 2 lines

2.4.3

Regards,
Simon

#2 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

Quoted 10 lines · Simon Glass
Quoted 10 lines
include/common.h +0 -0
index c48e5bc..c12f402 100644
@@ -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)
 
Quoted 11 lines

include/common.h int cpu_release(int nr, int argc, char * const argv[]); · line 1070

Quoted 9 lines
include/common.h +8 -0
+#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/

#3 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

In reply to Simon Glass
Quoted 13 lines · Simon Glass

Hi Marcel,

Quoted 10 lines · Marcel Ziswiler
include/common.h +0 -0
index c48e5bc..c12f402 100644
@@ -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)
 
Quoted 11 lines

include/common.h int cpu_release(int nr, int argc, char * const argv[]); · line 1070

Quoted 9 lines
include/common.h +8 -0
+#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

#4 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

Hi,

Quoted 26 lines · Scott Wood
Quoted 13 lines · Simon Glass

Hi Marcel,

Quoted 10 lines · Marcel Ziswiler
include/common.h +0 -0
index c48e5bc..c12f402 100644
@@ -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)
 
Quoted 11 lines

include/common.h int cpu_release(int nr, int argc, char * const argv[]); · line 1070

Quoted 9 lines
include/common.h +8 -0
+#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?

Quoted 2 lines

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

#5 lore raw

PATCH RESEND v6 1/1 mtd/nand/ubi: assortment of alignment fixes

In reply to Marcel Ziswiler
Quoted 11 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150828/64aea71d/attachment.sig>