We memset this page before checking whether it's valid. But we need
not memset zeroed page at all.
Signed-off-by: Guo Chao <[email protected]>
---
fs/ext4/super.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ad6cd8a..66a4e20 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3206,7 +3206,6 @@ int ext4_calculate_overhead(struct super_block *sb)
ext4_fsblk_t overhead = 0;
char *buf = (char *) get_zeroed_page(GFP_KERNEL);
- memset(buf, 0, PAGE_SIZE);
if (!buf)
return -ENOMEM;
--
1.7.9.5
We have a dedicated interface to sync inode metadata.
Signed-off-by: Guo Chao <[email protected]>
---
fs/ext4/fsync.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index be1d89f..dfbc1fe 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -44,7 +44,6 @@
*/
static int ext4_sync_parent(struct inode *inode)
{
- struct writeback_control wbc;
struct dentry *dentry = NULL;
struct inode *next;
int ret = 0;
@@ -66,10 +65,7 @@ static int ext4_sync_parent(struct inode *inode)
ret = sync_mapping_buffers(inode->i_mapping);
if (ret)
break;
- memset(&wbc, 0, sizeof(wbc));
- wbc.sync_mode = WB_SYNC_ALL;
- wbc.nr_to_write = 0; /* only write out the inode */
- ret = sync_inode(inode, &wbc);
+ ret = sync_inode_metadata(inode, 1);
if (ret)
break;
}
--
1.7.9.5
inode_init_always() will initialize inode->i_data.writeback_index
anyway, no need to do this in ext4_alloc_inode().
Signed-off-by: Guo Chao <[email protected]>
---
fs/ext4/super.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 66a4e20..15d28e3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -940,7 +940,6 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
return NULL;
ei->vfs_inode.i_version = 1;
- ei->vfs_inode.i_data.writeback_index = 0;
memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache));
INIT_LIST_HEAD(&ei->i_prealloc_list);
spin_lock_init(&ei->i_prealloc_lock);
--
1.7.9.5
We use kzalloc() to allocate sbi, no need to zero its field.
Signed-off-by: Guo Chao <[email protected]>
---
fs/ext4/super.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 15d28e3..a99f6c7 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3799,7 +3799,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
mutex_init(&sbi->s_orphan_lock);
- sbi->s_resize_flags = 0;
sb->s_root = NULL;
--
1.7.9.5
On Fri, 30 Nov 2012, Guo Chao wrote:
> Date: Fri, 30 Nov 2012 14:41:43 +0800
> From: Guo Chao <[email protected]>
> To: [email protected]
> Cc: [email protected]
> Subject: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()
>
> We memset this page before checking whether it's valid. But we need
> not memset zeroed page at all.
>
> Signed-off-by: Guo Chao <[email protected]>
> ---
> fs/ext4/super.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index ad6cd8a..66a4e20 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3206,7 +3206,6 @@ int ext4_calculate_overhead(struct super_block *sb)
> ext4_fsblk_t overhead = 0;
> char *buf = (char *) get_zeroed_page(GFP_KERNEL);
>
> - memset(buf, 0, PAGE_SIZE);
> if (!buf)
> return -ENOMEM;
Good catch, thanks!
Reviewed-by: Lukas Czerner <[email protected]>
On Fri, 30 Nov 2012, Guo Chao wrote:
> Date: Fri, 30 Nov 2012 14:41:44 +0800
> From: Guo Chao <[email protected]>
> To: [email protected]
> Cc: [email protected]
> Subject: [PATCH 2/4] ext4: use sync_inode_metadata() when sync inode metadata
>
> We have a dedicated interface to sync inode metadata.
>
> Signed-off-by: Guo Chao <[email protected]>
> ---
> fs/ext4/fsync.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
> index be1d89f..dfbc1fe 100644
> --- a/fs/ext4/fsync.c
> +++ b/fs/ext4/fsync.c
> @@ -44,7 +44,6 @@
> */
> static int ext4_sync_parent(struct inode *inode)
> {
> - struct writeback_control wbc;
> struct dentry *dentry = NULL;
> struct inode *next;
> int ret = 0;
> @@ -66,10 +65,7 @@ static int ext4_sync_parent(struct inode *inode)
> ret = sync_mapping_buffers(inode->i_mapping);
> if (ret)
> break;
> - memset(&wbc, 0, sizeof(wbc));
> - wbc.sync_mode = WB_SYNC_ALL;
> - wbc.nr_to_write = 0; /* only write out the inode */
> - ret = sync_inode(inode, &wbc);
> + ret = sync_inode_metadata(inode, 1);
> if (ret)
> break;
> }
>
Makes sense.
Reviewed-by: Lukas Czerner <[email protected]>
On Fri, 30 Nov 2012, Guo Chao wrote:
> Date: Fri, 30 Nov 2012 14:41:45 +0800
> From: Guo Chao <[email protected]>
> To: [email protected]
> Cc: [email protected]
> Subject: [PATCH 3/4] ext4: remove redundant code in ext4_alloc_inode()
>
> inode_init_always() will initialize inode->i_data.writeback_index
> anyway, no need to do this in ext4_alloc_inode().
>
> Signed-off-by: Guo Chao <[email protected]>
> ---
> fs/ext4/super.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 66a4e20..15d28e3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -940,7 +940,6 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
> return NULL;
>
> ei->vfs_inode.i_version = 1;
> - ei->vfs_inode.i_data.writeback_index = 0;
> memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache));
> INIT_LIST_HEAD(&ei->i_prealloc_list);
> spin_lock_init(&ei->i_prealloc_lock);
>
Looks good
Reviewed-by: Lukas Czerner <[email protected]>
On Fri, 30 Nov 2012, Guo Chao wrote:
> Date: Fri, 30 Nov 2012 14:41:46 +0800
> From: Guo Chao <[email protected]>
> To: [email protected]
> Cc: [email protected]
> Subject: [PATCH 4/4] ext4: remove redundant initialization in
> ext4_fill_super()
>
> We use kzalloc() to allocate sbi, no need to zero its field.
>
> Signed-off-by: Guo Chao <[email protected]>
> ---
> fs/ext4/super.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 15d28e3..a99f6c7 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3799,7 +3799,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>
> INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
> mutex_init(&sbi->s_orphan_lock);
> - sbi->s_resize_flags = 0;
>
> sb->s_root = NULL;
>
>
There are other members of sbi which we initialize to zero even
though we use kzalloc.
Thanks!
-Lukas
On Fri, 30 Nov 2012, Luk?? Czerner wrote:
> Date: Fri, 30 Nov 2012 14:20:42 +0100 (CET)
> From: Luk?? Czerner <[email protected]>
> To: Guo Chao <[email protected]>
> Cc: [email protected], [email protected]
> Subject: Re: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()
>
> On Fri, 30 Nov 2012, Guo Chao wrote:
>
> > Date: Fri, 30 Nov 2012 14:41:43 +0800
> > From: Guo Chao <[email protected]>
> > To: [email protected]
> > Cc: [email protected]
> > Subject: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()
> >
> > We memset this page before checking whether it's valid. But we need
> > not memset zeroed page at all.
> >
> > Signed-off-by: Guo Chao <[email protected]>
> > ---
> > fs/ext4/super.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index ad6cd8a..66a4e20 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -3206,7 +3206,6 @@ int ext4_calculate_overhead(struct super_block *sb)
> > ext4_fsblk_t overhead = 0;
> > char *buf = (char *) get_zeroed_page(GFP_KERNEL);
> >
> > - memset(buf, 0, PAGE_SIZE);
> > if (!buf)
> > return -ENOMEM;
>
> Good catch, thanks!
>
> Reviewed-by: Lukas Czerner <[email protected]>
It looks like that it has been already fixed with a different patch.
http://www.spinics.net/lists/linux-ext4/msg35310.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Fri, Nov 30, 2012 at 03:04:59PM +0100, Lukáš Czerner wrote:
> On Fri, 30 Nov 2012, Lukáš Czerner wrote:
> It looks like that it has been already fixed with a different patch.
>
> http://www.spinics.net/lists/linux-ext4/msg35310.html
>
Hmm ... I did search this function in marc.info. Looks like I should
switch to another database. Thank you for review.
Thanks,
Guo Chao
On Fri, Nov 30, 2012 at 02:41:44PM +0800, Guo Chao wrote:
> We have a dedicated interface to sync inode metadata.
>
> Signed-off-by: Guo Chao <[email protected]>
Thanks, applied.
- Ted
On Fri, Nov 30, 2012 at 02:41:45PM +0800, Guo Chao wrote:
> inode_init_always() will initialize inode->i_data.writeback_index
> anyway, no need to do this in ext4_alloc_inode().
>
> Signed-off-by: Guo Chao <[email protected]>
Thanks, applied.
- Ted
On Fri, Nov 30, 2012 at 02:41:46PM +0800, Guo Chao wrote:
> We use kzalloc() to allocate sbi, no need to zero its field.
>
> Signed-off-by: Guo Chao <[email protected]>
Thanks, applied.
- Ted