| Message ID | 20180922021335.31265-1-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | c9795396edd53ad29537037de8fd752662c676ad |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] fs: btrfs: Fix cache alignment bugs | expand |
On Sat, Sep 22, 2018 at 04:13:35AM +0200, Marek Vasut wrote: > The btrfs implementation passes cache-unaligned buffers into the > block layer, which triggers cache alignment problems down in the > block device drivers. Align the buffers to prevent this. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Marek Behun <marek.behun@nic.cz> Applied to u-boot/master, thanks!
Tested-by: Marek Behún <marek.behun@nic.cz> On Sat, 22 Sep 2018 04:13:35 +0200 Marek Vasut <marex@denx.de> wrote: > The btrfs implementation passes cache-unaligned buffers into the > block layer, which triggers cache alignment problems down in the > block device drivers. Align the buffers to prevent this. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Marek Behun <marek.behun@nic.cz> > --- > fs/btrfs/ctree.c | 24 +++++++++++++----------- > fs/btrfs/extent-io.c | 3 ++- > fs/btrfs/super.c | 3 ++- > 3 files changed, 17 insertions(+), 13 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 4da36a9bc7..18b47d92fe 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -7,6 +7,7 @@ > > #include "btrfs.h" > #include <malloc.h> > +#include <memalign.h> > > int btrfs_comp_keys(struct btrfs_key *a, struct btrfs_key *b) > { > @@ -105,23 +106,24 @@ void btrfs_free_path(struct btrfs_path *p) > > static int read_tree_node(u64 physical, union btrfs_tree_node **buf) > { > - struct btrfs_header hdr; > - unsigned long size, offset = sizeof(hdr); > + ALLOC_CACHE_ALIGN_BUFFER(struct btrfs_header, hdr, > + sizeof(struct btrfs_header)); > + unsigned long size, offset = sizeof(*hdr); > union btrfs_tree_node *res; > u32 i; > > - if (!btrfs_devread(physical, sizeof(hdr), &hdr)) > + if (!btrfs_devread(physical, sizeof(*hdr), hdr)) > return -1; > > - btrfs_header_to_cpu(&hdr); > + btrfs_header_to_cpu(hdr); > > - if (hdr.level) > + if (hdr->level) > size = sizeof(struct btrfs_node) > - + hdr.nritems * sizeof(struct btrfs_key_ptr); > + + hdr->nritems * sizeof(struct btrfs_key_ptr); > else > size = btrfs_info.sb.nodesize; > > - res = malloc(size); > + res = malloc_cache_aligned(size); > if (!res) { > debug("%s: malloc failed\n", __func__); > return -1; > @@ -133,12 +135,12 @@ static int read_tree_node(u64 physical, union > btrfs_tree_node **buf) return -1; > } > > - res->header = hdr; > - if (hdr.level) > - for (i = 0; i < hdr.nritems; ++i) > + memcpy(&res->header, hdr, sizeof(*hdr)); > + if (hdr->level) > + for (i = 0; i < hdr->nritems; ++i) > btrfs_key_ptr_to_cpu(&res->node.ptrs[i]); > else > - for (i = 0; i < hdr.nritems; ++i) > + for (i = 0; i < hdr->nritems; ++i) > btrfs_item_to_cpu(&res->leaf.items[i]); > > *buf = res; > diff --git a/fs/btrfs/extent-io.c b/fs/btrfs/extent-io.c > index 7263f41644..66d0e1c7d6 100644 > --- a/fs/btrfs/extent-io.c > +++ b/fs/btrfs/extent-io.c > @@ -7,6 +7,7 @@ > > #include "btrfs.h" > #include <malloc.h> > +#include <memalign.h> > > u64 btrfs_read_extent_inline(struct btrfs_path *path, > struct btrfs_file_extent_item *extent, > u64 offset, @@ -89,7 +90,7 @@ u64 btrfs_read_extent_reg(struct > btrfs_path *path, return size; > } > > - cbuf = malloc(dlen > size ? clen + dlen : clen); > + cbuf = malloc_cache_aligned(dlen > size ? clen + dlen : > clen); if (!cbuf) > return -1ULL; > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index e680caa56a..7aaf8f9b0d 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -6,6 +6,7 @@ > */ > > #include "btrfs.h" > +#include <memalign.h> > > #define BTRFS_SUPER_FLAG_SUPP > (BTRFS_HEADER_FLAG_WRITTEN \ | BTRFS_HEADER_FLAG_RELOC \ > @@ -179,7 +180,7 @@ int btrfs_read_superblock(void) > 0x4000000000ull, > 0x4000000000000ull > }; > - char raw_sb[BTRFS_SUPER_INFO_SIZE]; > + ALLOC_CACHE_ALIGN_BUFFER(char, raw_sb, > BTRFS_SUPER_INFO_SIZE); struct btrfs_super_block *sb = (struct > btrfs_super_block *) raw_sb; u64 dev_total_bytes; > int i;
On 10/02/2018 01:27 PM, Marek Behún wrote: > Tested-by: Marek Behún <marek.behun@nic.cz> btw don't you see those warnings/problems on the Armada platform ? I presume this is mostly used on the Omnia. Maybe the cache alignment checking there is not as verbose as on other platforms. > On Sat, 22 Sep 2018 04:13:35 +0200 > Marek Vasut <marex@denx.de> wrote: > >> The btrfs implementation passes cache-unaligned buffers into the >> block layer, which triggers cache alignment problems down in the >> block device drivers. Align the buffers to prevent this. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Marek Behun <marek.behun@nic.cz> >> --- >> fs/btrfs/ctree.c | 24 +++++++++++++----------- >> fs/btrfs/extent-io.c | 3 ++- >> fs/btrfs/super.c | 3 ++- >> 3 files changed, 17 insertions(+), 13 deletions(-) >> >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c >> index 4da36a9bc7..18b47d92fe 100644 >> --- a/fs/btrfs/ctree.c >> +++ b/fs/btrfs/ctree.c >> @@ -7,6 +7,7 @@ >> >> #include "btrfs.h" >> #include <malloc.h> >> +#include <memalign.h> >> >> int btrfs_comp_keys(struct btrfs_key *a, struct btrfs_key *b) >> { >> @@ -105,23 +106,24 @@ void btrfs_free_path(struct btrfs_path *p) >> >> static int read_tree_node(u64 physical, union btrfs_tree_node **buf) >> { >> - struct btrfs_header hdr; >> - unsigned long size, offset = sizeof(hdr); >> + ALLOC_CACHE_ALIGN_BUFFER(struct btrfs_header, hdr, >> + sizeof(struct btrfs_header)); >> + unsigned long size, offset = sizeof(*hdr); >> union btrfs_tree_node *res; >> u32 i; >> >> - if (!btrfs_devread(physical, sizeof(hdr), &hdr)) >> + if (!btrfs_devread(physical, sizeof(*hdr), hdr)) >> return -1; >> >> - btrfs_header_to_cpu(&hdr); >> + btrfs_header_to_cpu(hdr); >> >> - if (hdr.level) >> + if (hdr->level) >> size = sizeof(struct btrfs_node) >> - + hdr.nritems * sizeof(struct btrfs_key_ptr); >> + + hdr->nritems * sizeof(struct btrfs_key_ptr); >> else >> size = btrfs_info.sb.nodesize; >> >> - res = malloc(size); >> + res = malloc_cache_aligned(size); >> if (!res) { >> debug("%s: malloc failed\n", __func__); >> return -1; >> @@ -133,12 +135,12 @@ static int read_tree_node(u64 physical, union >> btrfs_tree_node **buf) return -1; >> } >> >> - res->header = hdr; >> - if (hdr.level) >> - for (i = 0; i < hdr.nritems; ++i) >> + memcpy(&res->header, hdr, sizeof(*hdr)); >> + if (hdr->level) >> + for (i = 0; i < hdr->nritems; ++i) >> btrfs_key_ptr_to_cpu(&res->node.ptrs[i]); >> else >> - for (i = 0; i < hdr.nritems; ++i) >> + for (i = 0; i < hdr->nritems; ++i) >> btrfs_item_to_cpu(&res->leaf.items[i]); >> >> *buf = res; >> diff --git a/fs/btrfs/extent-io.c b/fs/btrfs/extent-io.c >> index 7263f41644..66d0e1c7d6 100644 >> --- a/fs/btrfs/extent-io.c >> +++ b/fs/btrfs/extent-io.c >> @@ -7,6 +7,7 @@ >> >> #include "btrfs.h" >> #include <malloc.h> >> +#include <memalign.h> >> >> u64 btrfs_read_extent_inline(struct btrfs_path *path, >> struct btrfs_file_extent_item *extent, >> u64 offset, @@ -89,7 +90,7 @@ u64 btrfs_read_extent_reg(struct >> btrfs_path *path, return size; >> } >> >> - cbuf = malloc(dlen > size ? clen + dlen : clen); >> + cbuf = malloc_cache_aligned(dlen > size ? clen + dlen : >> clen); if (!cbuf) >> return -1ULL; >> >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c >> index e680caa56a..7aaf8f9b0d 100644 >> --- a/fs/btrfs/super.c >> +++ b/fs/btrfs/super.c >> @@ -6,6 +6,7 @@ >> */ >> >> #include "btrfs.h" >> +#include <memalign.h> >> >> #define BTRFS_SUPER_FLAG_SUPP >> (BTRFS_HEADER_FLAG_WRITTEN \ | BTRFS_HEADER_FLAG_RELOC \ >> @@ -179,7 +180,7 @@ int btrfs_read_superblock(void) >> 0x4000000000ull, >> 0x4000000000000ull >> }; >> - char raw_sb[BTRFS_SUPER_INFO_SIZE]; >> + ALLOC_CACHE_ALIGN_BUFFER(char, raw_sb, >> BTRFS_SUPER_INFO_SIZE); struct btrfs_super_block *sb = (struct >> btrfs_super_block *) raw_sb; u64 dev_total_bytes; >> int i; >
On Tue, 2 Oct 2018 13:43:55 +0200 Marek Vasut <marex@denx.de> wrote: > On 10/02/2018 01:27 PM, Marek Behún wrote: > > Tested-by: Marek Behún <marek.behun@nic.cz> > > btw don't you see those warnings/problems on the Armada platform ? I > presume this is mostly used on the Omnia. Maybe the cache alignment > checking there is not as verbose as on other platforms. Hi, no, I cannot say that I had problem with that (but maybe I don't remember), but the last 12 months I have been working on 64bit Armada, so maybe the problem does not present on AArch64. Marek
On 10/02/2018 02:08 PM, Marek Behún wrote: > On Tue, 2 Oct 2018 13:43:55 +0200 > Marek Vasut <marex@denx.de> wrote: > >> On 10/02/2018 01:27 PM, Marek Behún wrote: >>> Tested-by: Marek Behún <marek.behun@nic.cz> >> >> btw don't you see those warnings/problems on the Armada platform ? I >> presume this is mostly used on the Omnia. Maybe the cache alignment >> checking there is not as verbose as on other platforms. > > Hi, no, I cannot say that I had problem with that (but maybe I don't > remember), but the last 12 months I have been working on 64bit Armada, > so maybe the problem does not present on AArch64. It is present on all systems with data cache enabled and with block drivers that do DMA, since those structures are not aligned.
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 4da36a9bc7..18b47d92fe 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -7,6 +7,7 @@ #include "btrfs.h" #include <malloc.h> +#include <memalign.h> int btrfs_comp_keys(struct btrfs_key *a, struct btrfs_key *b) { @@ -105,23 +106,24 @@ void btrfs_free_path(struct btrfs_path *p) static int read_tree_node(u64 physical, union btrfs_tree_node **buf) { - struct btrfs_header hdr; - unsigned long size, offset = sizeof(hdr); + ALLOC_CACHE_ALIGN_BUFFER(struct btrfs_header, hdr, + sizeof(struct btrfs_header)); + unsigned long size, offset = sizeof(*hdr); union btrfs_tree_node *res; u32 i; - if (!btrfs_devread(physical, sizeof(hdr), &hdr)) + if (!btrfs_devread(physical, sizeof(*hdr), hdr)) return -1; - btrfs_header_to_cpu(&hdr); + btrfs_header_to_cpu(hdr); - if (hdr.level) + if (hdr->level) size = sizeof(struct btrfs_node) - + hdr.nritems * sizeof(struct btrfs_key_ptr); + + hdr->nritems * sizeof(struct btrfs_key_ptr); else size = btrfs_info.sb.nodesize; - res = malloc(size); + res = malloc_cache_aligned(size); if (!res) { debug("%s: malloc failed\n", __func__); return -1; @@ -133,12 +135,12 @@ static int read_tree_node(u64 physical, union btrfs_tree_node **buf) return -1; } - res->header = hdr; - if (hdr.level) - for (i = 0; i < hdr.nritems; ++i) + memcpy(&res->header, hdr, sizeof(*hdr)); + if (hdr->level) + for (i = 0; i < hdr->nritems; ++i) btrfs_key_ptr_to_cpu(&res->node.ptrs[i]); else - for (i = 0; i < hdr.nritems; ++i) + for (i = 0; i < hdr->nritems; ++i) btrfs_item_to_cpu(&res->leaf.items[i]); *buf = res; diff --git a/fs/btrfs/extent-io.c b/fs/btrfs/extent-io.c index 7263f41644..66d0e1c7d6 100644 --- a/fs/btrfs/extent-io.c +++ b/fs/btrfs/extent-io.c @@ -7,6 +7,7 @@ #include "btrfs.h" #include <malloc.h> +#include <memalign.h> u64 btrfs_read_extent_inline(struct btrfs_path *path, struct btrfs_file_extent_item *extent, u64 offset, @@ -89,7 +90,7 @@ u64 btrfs_read_extent_reg(struct btrfs_path *path, return size; } - cbuf = malloc(dlen > size ? clen + dlen : clen); + cbuf = malloc_cache_aligned(dlen > size ? clen + dlen : clen); if (!cbuf) return -1ULL; diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index e680caa56a..7aaf8f9b0d 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -6,6 +6,7 @@ */ #include "btrfs.h" +#include <memalign.h> #define BTRFS_SUPER_FLAG_SUPP (BTRFS_HEADER_FLAG_WRITTEN \ | BTRFS_HEADER_FLAG_RELOC \ @@ -179,7 +180,7 @@ int btrfs_read_superblock(void) 0x4000000000ull, 0x4000000000000ull }; - char raw_sb[BTRFS_SUPER_INFO_SIZE]; + ALLOC_CACHE_ALIGN_BUFFER(char, raw_sb, BTRFS_SUPER_INFO_SIZE); struct btrfs_super_block *sb = (struct btrfs_super_block *) raw_sb; u64 dev_total_bytes; int i;
The btrfs implementation passes cache-unaligned buffers into the block layer, which triggers cache alignment problems down in the block device drivers. Align the buffers to prevent this. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Marek Behun <marek.behun@nic.cz> --- fs/btrfs/ctree.c | 24 +++++++++++++----------- fs/btrfs/extent-io.c | 3 ++- fs/btrfs/super.c | 3 ++- 3 files changed, 17 insertions(+), 13 deletions(-)