[PATCH 1/4] ext4: remove unsafe and unnecessary memset()

[PATCH 1/4] ext4: remove unsafe and unnecessary memset()

[Guo Chao]

We memset this page before checking whether it's valid. But we need
not memset zeroed page at all.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 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

[PATCH 2/4] ext4: use sync_inode_metadata() when sync inode metadata

[Guo Chao]

We have a dedicated interface to sync inode metadata.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 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

[PATCH 3/4] ext4: remove redundant code in ext4_alloc_inode()

[Guo Chao]

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 <yan@linux.vnet.ibm.com>
---
 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

[PATCH 4/4] ext4: remove redundant initialization in ext4_fill_super()

[Guo Chao]

We use kzalloc() to allocate sbi, no need to zero its field.

Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
 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

Re: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()

[Lukáš Czerner]

On Fri, 30 Nov 2012, Guo Chao wrote:

> Date: Fri, 30 Nov 2012 14:41:43 +0800
> From: Guo Chao <yan@linux.vnet.ibm.com>
> To: tytso@mit.edu
> Cc: linux-ext4@vger.kernel.org
> 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 <yan@linux.vnet.ibm.com>
> ---
>  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 <lczerner@redhat.com>

Re: [PATCH 2/4] ext4: use sync_inode_metadata() when sync inode metadata

[Lukáš Czerner]

On Fri, 30 Nov 2012, Guo Chao wrote:

> Date: Fri, 30 Nov 2012 14:41:44 +0800
> From: Guo Chao <yan@linux.vnet.ibm.com>
> To: tytso@mit.edu
> Cc: linux-ext4@vger.kernel.org
> 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 <yan@linux.vnet.ibm.com>
> ---
>  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 <lczerner@redhat.com>

Re: [PATCH 3/4] ext4: remove redundant code in ext4_alloc_inode()

[Lukáš Czerner]

On Fri, 30 Nov 2012, Guo Chao wrote:

> Date: Fri, 30 Nov 2012 14:41:45 +0800
> From: Guo Chao <yan@linux.vnet.ibm.com>
> To: tytso@mit.edu
> Cc: linux-ext4@vger.kernel.org
> 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 <yan@linux.vnet.ibm.com>
> ---
>  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 <lczerner@redhat.com>

Re: [PATCH 4/4] ext4: remove redundant initialization in ext4_fill_super()

[Lukáš Czerner]

On Fri, 30 Nov 2012, Guo Chao wrote:

> Date: Fri, 30 Nov 2012 14:41:46 +0800
> From: Guo Chao <yan@linux.vnet.ibm.com>
> To: tytso@mit.edu
> Cc: linux-ext4@vger.kernel.org
> 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 <yan@linux.vnet.ibm.com>
> ---
>  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

Re: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1590 bytes --]

On Fri, 30 Nov 2012, Lukáš Czerner wrote:

> Date: Fri, 30 Nov 2012 14:20:42 +0100 (CET)
> From: Lukáš Czerner <lczerner@redhat.com>
> To: Guo Chao <yan@linux.vnet.ibm.com>
> Cc: tytso@mit.edu, linux-ext4@vger.kernel.org
> 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 <yan@linux.vnet.ibm.com>
> > To: tytso@mit.edu
> > Cc: linux-ext4@vger.kernel.org
> > 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 <yan@linux.vnet.ibm.com>
> > ---
> >  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 <lczerner@redhat.com>

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/4] ext4: remove unsafe and unnecessary memset()

[Guo Chao]

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

--
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

Re: [PATCH 2/4] ext4: use sync_inode_metadata() when sync inode metadata

[Theodore Ts'o]

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 <yan@linux.vnet.ibm.com>

Thanks, applied.

						- Ted

Re: [PATCH 3/4] ext4: remove redundant code in ext4_alloc_inode()

[Theodore Ts'o]

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 <yan@linux.vnet.ibm.com>

Thanks, applied.

					- Ted

Re: [PATCH 4/4] ext4: remove redundant initialization in ext4_fill_super()

[Theodore Ts'o]

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 <yan@linux.vnet.ibm.com>

Thanks, applied.

					- Ted