* [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks
@ 2013-07-12 1:00 Zheng Liu
2013-07-29 15:09 ` Zheng Liu
0 siblings, 1 reply; 4+ messages in thread
From: Zheng Liu @ 2013-07-12 1:00 UTC (permalink / raw)
To: linux-ext4; +Cc: Theodore Ts'o, Zheng Liu
From: Zheng Liu <wenqing.lz@taobao.com>
In this commit (921f266b) a sanity check is added in map_blocks to make
sure 'retval == map->m_len'. But we'd better enable this check all the
time because it can help us to dig some bugs that is hard to be hitted.
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
changelog:
v2 <- v1:
* print more details to help us to dig some bugs that is hard to be hitted
v1: http://patchwork.ozlabs.org/patch/230406/
Hi Ted,
This patch sleeps on my own tree for a very long time so that I almost forgot
it. Sorry about that.
- Zheng
fs/ext4/inode.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 19a1643..95bad70 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -556,14 +556,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertation failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (lookup)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertation failed for inode: "
+ "%lu retval %d != map->m_len %d\n",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
status = map->m_flags & EXT4_MAP_UNWRITTEN ?
EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
@@ -657,14 +656,13 @@ found:
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertation failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (allocation)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertation failed for inode: "
+ "%lu retval %d != map->m_len %d\n",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
/*
* If the extent has been zeroed out, we don't need to update
@@ -1640,14 +1638,13 @@ add_delayed:
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertation failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (lookup)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertation failed for inode: "
+ "%lu retval %d != map->m_len %d\n",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
status = map->m_flags & EXT4_MAP_UNWRITTEN ?
EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
--
1.7.9.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks
2013-07-12 1:00 [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks Zheng Liu
@ 2013-07-29 15:09 ` Zheng Liu
2013-07-29 16:52 ` Theodore Ts'o
0 siblings, 1 reply; 4+ messages in thread
From: Zheng Liu @ 2013-07-29 15:09 UTC (permalink / raw)
To: linux-ext4, Theodore Ts'o; +Cc: Zheng Liu
On Fri, Jul 12, 2013 at 09:00:52AM +0800, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@taobao.com>
>
> In this commit (921f266b) a sanity check is added in map_blocks to make
> sure 'retval == map->m_len'. But we'd better enable this check all the
> time because it can help us to dig some bugs that is hard to be hitted.
>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
Any comment?
Thanks
- Zheng
> ---
> changelog:
> v2 <- v1:
> * print more details to help us to dig some bugs that is hard to be hitted
>
> v1: http://patchwork.ozlabs.org/patch/230406/
>
> Hi Ted,
>
> This patch sleeps on my own tree for a very long time so that I almost forgot
> it. Sorry about that.
>
> - Zheng
>
> fs/ext4/inode.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 19a1643..95bad70 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -556,14 +556,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertation failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (lookup)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertation failed for inode: "
> + "%lu retval %d != map->m_len %d\n",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> @@ -657,14 +656,13 @@ found:
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertation failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (allocation)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertation failed for inode: "
> + "%lu retval %d != map->m_len %d\n",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> /*
> * If the extent has been zeroed out, we don't need to update
> @@ -1640,14 +1638,13 @@ add_delayed:
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertation failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (lookup)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertation failed for inode: "
> + "%lu retval %d != map->m_len %d\n",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> --
> 1.7.9.7
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks
2013-07-29 15:09 ` Zheng Liu
@ 2013-07-29 16:52 ` Theodore Ts'o
2013-07-29 23:37 ` Zheng Liu
0 siblings, 1 reply; 4+ messages in thread
From: Theodore Ts'o @ 2013-07-29 16:52 UTC (permalink / raw)
To: linux-ext4, Zheng Liu
The patch didn't apply as-is so I had to adjust it a bit. I also
reworded the commit description.
- Ted
>From 44fb851dfb2f8e3462617e19a7b3b9025db9d919 Mon Sep 17 00:00:00 2001
From: Zheng Liu <wenqing.lz@taobao.com>
Date: Mon, 29 Jul 2013 12:51:42 -0400
Subject: [PATCH] ext4: add WARN_ON to check the length of allocated blocks
In commit 921f266b: ext4: add self-testing infrastructure to do a
sanity check, some sanity checks were added in map_blocks to make sure
'retval == map->m_len'.
Enable these checks by default and report any assertion failures using
ext4_warning() and WARN_ON() since they can help us to figure out some
bugs that are otherwise hard to hit.
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
fs/ext4/inode.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ba33c67..dd32a2e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -555,14 +555,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertion failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (lookup)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertion failed for inode "
+ "%lu: retval %d != map->m_len %d",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
status = map->m_flags & EXT4_MAP_UNWRITTEN ?
EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
@@ -656,14 +655,13 @@ found:
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertion failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (allocation)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertion failed for inode "
+ "%lu: retval %d != map->m_len %d",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
/*
* If the extent has been zeroed out, we don't need to update
@@ -1637,14 +1635,13 @@ add_delayed:
int ret;
unsigned long long status;
-#ifdef ES_AGGRESSIVE_TEST
- if (retval != map->m_len) {
- printk("ES len assertion failed for inode: %lu "
- "retval %d != map->m_len %d "
- "in %s (lookup)\n", inode->i_ino, retval,
- map->m_len, __func__);
+ if (unlikely(retval != map->m_len)) {
+ ext4_warning(inode->i_sb,
+ "ES len assertion failed for inode "
+ "%lu: retval %d != map->m_len %d",
+ inode->i_ino, retval, map->m_len);
+ WARN_ON(1);
}
-#endif
status = map->m_flags & EXT4_MAP_UNWRITTEN ?
EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks
2013-07-29 16:52 ` Theodore Ts'o
@ 2013-07-29 23:37 ` Zheng Liu
0 siblings, 0 replies; 4+ messages in thread
From: Zheng Liu @ 2013-07-29 23:37 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, Zheng Liu
On Mon, Jul 29, 2013 at 12:52:45PM -0400, Theodore Ts'o wrote:
> The patch didn't apply as-is so I had to adjust it a bit. I also
> reworded the commit description.
Ah, sorry, I forgot that this patch needs to be rebased against the
latest ext4/dev branch. The patch looks good. Thanks for fixing it.
- Zheng
>
> - Ted
>
> From 44fb851dfb2f8e3462617e19a7b3b9025db9d919 Mon Sep 17 00:00:00 2001
> From: Zheng Liu <wenqing.lz@taobao.com>
> Date: Mon, 29 Jul 2013 12:51:42 -0400
> Subject: [PATCH] ext4: add WARN_ON to check the length of allocated blocks
>
> In commit 921f266b: ext4: add self-testing infrastructure to do a
> sanity check, some sanity checks were added in map_blocks to make sure
> 'retval == map->m_len'.
>
> Enable these checks by default and report any assertion failures using
> ext4_warning() and WARN_ON() since they can help us to figure out some
> bugs that are otherwise hard to hit.
>
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> fs/ext4/inode.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ba33c67..dd32a2e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -555,14 +555,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertion failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (lookup)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertion failed for inode "
> + "%lu: retval %d != map->m_len %d",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> @@ -656,14 +655,13 @@ found:
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertion failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (allocation)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertion failed for inode "
> + "%lu: retval %d != map->m_len %d",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> /*
> * If the extent has been zeroed out, we don't need to update
> @@ -1637,14 +1635,13 @@ add_delayed:
> int ret;
> unsigned long long status;
>
> -#ifdef ES_AGGRESSIVE_TEST
> - if (retval != map->m_len) {
> - printk("ES len assertion failed for inode: %lu "
> - "retval %d != map->m_len %d "
> - "in %s (lookup)\n", inode->i_ino, retval,
> - map->m_len, __func__);
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertion failed for inode "
> + "%lu: retval %d != map->m_len %d",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> }
> -#endif
>
> status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> --
> 1.7.12.rc0.22.gcdd159b
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-29 23:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-12 1:00 [PATCH v2] ext4: add WARN_ON to check the length of allocated blocks Zheng Liu
2013-07-29 15:09 ` Zheng Liu
2013-07-29 16:52 ` Theodore Ts'o
2013-07-29 23:37 ` Zheng Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).