From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: Zhiguo Niu <zhiguo.niu@unisoc.com>,
linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix unaligned field offset in 32-bits platform
Date: Thu, 9 Mar 2023 15:25:49 -0800 [thread overview]
Message-ID: <ZApq/Rg2sbkCuQ2i@google.com> (raw)
In-Reply-To: <20230307151408.58490-1-chao@kernel.org>
On 03/07, Chao Yu wrote:
> F2FS-fs (dm-x): inconsistent rbtree, cur(3470333575168) next(3320009719808)
> ------------[ cut here ]------------
> kernel BUG at fs/f2fs/gc.c:602!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> PC is at get_victim_by_default+0x13c0/0x1498
> LR is at f2fs_check_rb_tree_consistence+0xc4/0xd4
> ....
> [<c04d98b0>] (get_victim_by_default) from [<c04d4f44>] (f2fs_gc+0x220/0x6cc)
> [<c04d4f44>] (f2fs_gc) from [<c04d4780>] (gc_thread_func+0x2ac/0x708)
> [<c04d4780>] (gc_thread_func) from [<c015c774>] (kthread+0x1a8/0x1b4)
> [<c015c774>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
>
> The reason is there is __packed attribute in struct rb_entry, but there
> is no __packed attribute in struct victim_entry, so wrong offset of key
> field will be parsed in struct rb_entry in f2fs_check_rb_tree_consistence,
> it describes memory layouts of struct rb_entry and struct victim_entry in
> 32-bits platform as below:
>
> struct rb_entry {
> [0] struct rb_node rb_node;
> union {
> struct {...};
> [12] unsigned long long key;
> } __packed;
> }
> size of struct rb_entry: 20
>
> struct victim_entry {
> [0] struct rb_node rb_node;
> union {
> struct {...};
> [16] struct victim_info vi;
Why 16?
Don't we get the key from "unsigned long long mtime" starting at [12]?
struct rb_node rb_node; /* rb node located in rb-tree */
union {
struct {
unsigned long long mtime; /* mtime of section */
unsigned int segno; /* segment No. */
};
struct victim_info vi; /* victim info */
};
> };
> [32] struct list_head list;
> }
> size of struct victim_entry: 40
>
> This patch tries to add __packed attribute in below structure:
> - discard_info, discard_cmd
> - extent_info, extent_node
> - victim_info, victim_entry
> in order to fix this unaligned field offset issue in 32-bits platform.
>
> Fixes: 004b68621897 ("f2fs: use rb-tree to track pending discard commands")
> Fixes: 13054c548a1c ("f2fs: introduce infra macro and data structure of rb-tree extent cache")
> Fixes: 093749e296e2 ("f2fs: support age threshold based garbage collection")
> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/f2fs.h | 6 +++---
> fs/f2fs/gc.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b0ab2062038a..17fa7572ceed 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -349,7 +349,7 @@ struct discard_info {
> block_t lstart; /* logical start address */
> block_t len; /* length */
> block_t start; /* actual start address in dev */
> -};
> +} __packed;
>
> struct discard_cmd {
> struct rb_node rb_node; /* rb node located in rb-tree */
> @@ -361,7 +361,7 @@ struct discard_cmd {
> };
> struct discard_info di; /* discard info */
>
> - };
> + } __packed;
> struct list_head list; /* command list */
> struct completion wait; /* compleation */
> struct block_device *bdev; /* bdev */
> @@ -660,7 +660,7 @@ struct extent_info {
> unsigned long long last_blocks;
> };
> };
> -};
> +} __packed;
>
> struct extent_node {
> struct rb_node rb_node; /* rb node located in rb-tree */
> diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
> index 15bd1d680f67..304937d9a084 100644
> --- a/fs/f2fs/gc.h
> +++ b/fs/f2fs/gc.h
> @@ -58,7 +58,7 @@ struct gc_inode_list {
> struct victim_info {
> unsigned long long mtime; /* mtime of section */
> unsigned int segno; /* section No. */
> -};
> +} __packed;
>
> struct victim_entry {
> struct rb_node rb_node; /* rb node located in rb-tree */
> @@ -68,7 +68,7 @@ struct victim_entry {
> unsigned int segno; /* segment No. */
> };
> struct victim_info vi; /* victim info */
> - };
> + } __packed;
> struct list_head list;
> };
>
> --
> 2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, Zhiguo Niu <zhiguo.niu@unisoc.com>
Subject: Re: [PATCH] f2fs: fix unaligned field offset in 32-bits platform
Date: Thu, 9 Mar 2023 15:25:49 -0800 [thread overview]
Message-ID: <ZApq/Rg2sbkCuQ2i@google.com> (raw)
In-Reply-To: <20230307151408.58490-1-chao@kernel.org>
On 03/07, Chao Yu wrote:
> F2FS-fs (dm-x): inconsistent rbtree, cur(3470333575168) next(3320009719808)
> ------------[ cut here ]------------
> kernel BUG at fs/f2fs/gc.c:602!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> PC is at get_victim_by_default+0x13c0/0x1498
> LR is at f2fs_check_rb_tree_consistence+0xc4/0xd4
> ....
> [<c04d98b0>] (get_victim_by_default) from [<c04d4f44>] (f2fs_gc+0x220/0x6cc)
> [<c04d4f44>] (f2fs_gc) from [<c04d4780>] (gc_thread_func+0x2ac/0x708)
> [<c04d4780>] (gc_thread_func) from [<c015c774>] (kthread+0x1a8/0x1b4)
> [<c015c774>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
>
> The reason is there is __packed attribute in struct rb_entry, but there
> is no __packed attribute in struct victim_entry, so wrong offset of key
> field will be parsed in struct rb_entry in f2fs_check_rb_tree_consistence,
> it describes memory layouts of struct rb_entry and struct victim_entry in
> 32-bits platform as below:
>
> struct rb_entry {
> [0] struct rb_node rb_node;
> union {
> struct {...};
> [12] unsigned long long key;
> } __packed;
> }
> size of struct rb_entry: 20
>
> struct victim_entry {
> [0] struct rb_node rb_node;
> union {
> struct {...};
> [16] struct victim_info vi;
Why 16?
Don't we get the key from "unsigned long long mtime" starting at [12]?
struct rb_node rb_node; /* rb node located in rb-tree */
union {
struct {
unsigned long long mtime; /* mtime of section */
unsigned int segno; /* segment No. */
};
struct victim_info vi; /* victim info */
};
> };
> [32] struct list_head list;
> }
> size of struct victim_entry: 40
>
> This patch tries to add __packed attribute in below structure:
> - discard_info, discard_cmd
> - extent_info, extent_node
> - victim_info, victim_entry
> in order to fix this unaligned field offset issue in 32-bits platform.
>
> Fixes: 004b68621897 ("f2fs: use rb-tree to track pending discard commands")
> Fixes: 13054c548a1c ("f2fs: introduce infra macro and data structure of rb-tree extent cache")
> Fixes: 093749e296e2 ("f2fs: support age threshold based garbage collection")
> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/f2fs.h | 6 +++---
> fs/f2fs/gc.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b0ab2062038a..17fa7572ceed 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -349,7 +349,7 @@ struct discard_info {
> block_t lstart; /* logical start address */
> block_t len; /* length */
> block_t start; /* actual start address in dev */
> -};
> +} __packed;
>
> struct discard_cmd {
> struct rb_node rb_node; /* rb node located in rb-tree */
> @@ -361,7 +361,7 @@ struct discard_cmd {
> };
> struct discard_info di; /* discard info */
>
> - };
> + } __packed;
> struct list_head list; /* command list */
> struct completion wait; /* compleation */
> struct block_device *bdev; /* bdev */
> @@ -660,7 +660,7 @@ struct extent_info {
> unsigned long long last_blocks;
> };
> };
> -};
> +} __packed;
>
> struct extent_node {
> struct rb_node rb_node; /* rb node located in rb-tree */
> diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
> index 15bd1d680f67..304937d9a084 100644
> --- a/fs/f2fs/gc.h
> +++ b/fs/f2fs/gc.h
> @@ -58,7 +58,7 @@ struct gc_inode_list {
> struct victim_info {
> unsigned long long mtime; /* mtime of section */
> unsigned int segno; /* section No. */
> -};
> +} __packed;
>
> struct victim_entry {
> struct rb_node rb_node; /* rb node located in rb-tree */
> @@ -68,7 +68,7 @@ struct victim_entry {
> unsigned int segno; /* segment No. */
> };
> struct victim_info vi; /* victim info */
> - };
> + } __packed;
> struct list_head list;
> };
>
> --
> 2.36.1
next prev parent reply other threads:[~2023-03-09 23:26 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-07 15:14 [f2fs-dev] [PATCH] f2fs: fix unaligned field offset in 32-bits platform Chao Yu
2023-03-07 15:14 ` Chao Yu
2023-03-07 17:26 ` [f2fs-dev] " Jaegeuk Kim
2023-03-07 17:26 ` Jaegeuk Kim
2023-03-08 1:31 ` [f2fs-dev] " Chao Yu
2023-03-08 1:31 ` Chao Yu
2023-03-07 17:40 ` [f2fs-dev] " patchwork-bot+f2fs
2023-03-07 17:40 ` patchwork-bot+f2fs
2023-03-08 10:16 ` David Laight
2023-03-08 10:16 ` David Laight
2023-03-09 23:54 ` [f2fs-dev] " Jaegeuk Kim
2023-03-09 23:54 ` Jaegeuk Kim
2023-03-10 9:28 ` [f2fs-dev] " David Laight
2023-03-10 9:28 ` David Laight
2023-03-10 21:08 ` [f2fs-dev] " Jaegeuk Kim
2023-03-10 21:08 ` Jaegeuk Kim
2023-03-09 23:25 ` Jaegeuk Kim [this message]
2023-03-09 23:25 ` Jaegeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZApq/Rg2sbkCuQ2i@google.com \
--to=jaegeuk@kernel.org \
--cc=chao@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=zhiguo.niu@unisoc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.