[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 11 Jul 2013 20:48:23 +0800
From: Zheng Liu <gnehzuil.liu@...il.com>
To: Theodore Ts'o <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
Zheng Liu <wenqing.lz@...bao.com>
Subject: Re: [RFC PATCH 2/3] ext4: use unsigned int for es_status values
Hi Ted,
On Thu, Jul 11, 2013 at 12:39:01AM -0400, Theodore Ts'o wrote:
> Don't use an unsigned long long for the es_status flags; this requires
> that we pass 64-bit values around which is painful on 32-bit systems.
> Instead pass the extent status flags around using the low 4 bits of an
> unsigned int, and shift them into place when we are reading or writing
> es_pblk.
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> Cc: Zheng Liu <wenqing.lz@...bao.com>
Thanks for fixing this. One minor nit below. Otherwise, the patch
looks good to me.
Reviewed-by: Zheng Liu <wenqing.lz@...bao.com>
> ---
> fs/ext4/extents_status.c | 6 +++---
> fs/ext4/extents_status.h | 47 ++++++++++++++++++++++++++-------------------
> fs/ext4/inode.c | 4 ++--
> include/trace/events/ext4.h | 14 +++++++-------
> 4 files changed, 39 insertions(+), 32 deletions(-)
>
[...]
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 19a1643..f27fa9d 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -554,7 +554,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> }
> if (retval > 0) {
> int ret;
> - unsigned long long status;
> + unsigned int status;
>
> #ifdef ES_AGGRESSIVE_TEST
> if (retval != map->m_len) {
In ext4_map_blocks() function, we have two places that need to be
change.
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f27fa9d..3cfbcaa 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -655,7 +655,7 @@ found:
if (retval > 0) {
int ret;
- unsigned long long status;
+ unsigned int status;
#ifdef ES_AGGRESSIVE_TEST
if (retval != map->m_len) {
Thanks,
- Zheng
> @@ -1638,7 +1638,7 @@ add_delayed:
> set_buffer_delay(bh);
> } else if (retval > 0) {
> int ret;
> - unsigned long long status;
> + unsigned int status;
>
> #ifdef ES_AGGRESSIVE_TEST
> if (retval != map->m_len) {
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index 2068db2..47a355b 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -64,10 +64,10 @@ struct extent_status;
> { EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER, "LAST_CLUSTER" })
>
> #define show_extent_status(status) __print_flags(status, "", \
> - { (1 << 3), "W" }, \
> - { (1 << 2), "U" }, \
> - { (1 << 1), "D" }, \
> - { (1 << 0), "H" })
> + { EXTENT_STATUS_WRITTEN, "W" }, \
> + { EXTENT_STATUS_UNWRITTEN, "U" }, \
> + { EXTENT_STATUS_DELAYED, "D" }, \
> + { EXTENT_STATUS_HOLE, "H" })
>
>
> TRACE_EVENT(ext4_free_inode,
> @@ -2212,7 +2212,7 @@ TRACE_EVENT(ext4_es_insert_extent,
> __entry->lblk = es->es_lblk;
> __entry->len = es->es_len;
> __entry->pblk = ext4_es_pblock(es);
> - __entry->status = ext4_es_status(es) >> 60;
> + __entry->status = ext4_es_status(es);
> ),
>
> TP_printk("dev %d,%d ino %lu es [%u/%u) mapped %llu status %s",
> @@ -2289,7 +2289,7 @@ TRACE_EVENT(ext4_es_find_delayed_extent_range_exit,
> __entry->lblk = es->es_lblk;
> __entry->len = es->es_len;
> __entry->pblk = ext4_es_pblock(es);
> - __entry->status = ext4_es_status(es) >> 60;
> + __entry->status = ext4_es_status(es);
> ),
>
> TP_printk("dev %d,%d ino %lu es [%u/%u) mapped %llu status %s",
> @@ -2343,7 +2343,7 @@ TRACE_EVENT(ext4_es_lookup_extent_exit,
> __entry->lblk = es->es_lblk;
> __entry->len = es->es_len;
> __entry->pblk = ext4_es_pblock(es);
> - __entry->status = ext4_es_status(es) >> 60;
> + __entry->status = ext4_es_status(es);
> __entry->found = found;
> ),
>
> --
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists