[1/3] btrfs: Initialize btrfs_io_ctl instead of memsetting it
diff mbox series

Message ID 20190225190744.21664-1-rgoldwyn@suse.de
State New
Headers show
Series
  • [1/3] btrfs: Initialize btrfs_io_ctl instead of memsetting it
Related show

Commit Message

Goldwyn Rodrigues Feb. 25, 2019, 7:07 p.m. UTC
From: Goldwyn Rodrigues <rgoldwyn@suse.com>

io_ctl_init() memsets it to zero anyways. However, I presume the
memset was added to avoid the WARN_ON in io_ctl_init().

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/free-space-cache.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

David Sterba Feb. 27, 2019, 3:47 p.m. UTC | #1
On Mon, Feb 25, 2019 at 01:07:42PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> io_ctl_init() memsets it to zero anyways. However, I presume the
> memset was added to avoid the WARN_ON in io_ctl_init().

I don't see any WARN_ON in io_ctl_init, you probably mean
__btrfs_write_out_cache.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/btrfs/free-space-cache.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 74aa552f4793..c813378ebf08 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -3544,13 +3544,12 @@ int btrfs_write_out_ino_cache(struct btrfs_root *root,
>  	struct btrfs_fs_info *fs_info = root->fs_info;
>  	struct btrfs_free_space_ctl *ctl = root->free_ino_ctl;
>  	int ret;
> -	struct btrfs_io_ctl io_ctl;
> +	struct btrfs_io_ctl io_ctl = {0};

Doesn't this zero the bytes unconditionally? The memset below happens
only when the inode cache is on.

>  	bool release_metadata = true;
>  
>  	if (!btrfs_test_opt(fs_info, INODE_MAP_CACHE))
>  		return 0;
>  
> -	memset(&io_ctl, 0, sizeof(io_ctl));
>  	ret = __btrfs_write_out_cache(root, inode, ctl, NULL, &io_ctl, trans);
>  	if (!ret) {
>  		/*
> -- 
> 2.16.4
Goldwyn Rodrigues Feb. 27, 2019, 5:02 p.m. UTC | #2
On 16:47 27/02, David Sterba wrote:
> On Mon, Feb 25, 2019 at 01:07:42PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > io_ctl_init() memsets it to zero anyways. However, I presume the
> > memset was added to avoid the WARN_ON in io_ctl_init().
> 
> I don't see any WARN_ON in io_ctl_init, you probably mean
> __btrfs_write_out_cache.

Yes, __btrfs_write_out_cache.

> > 
> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > ---
> >  fs/btrfs/free-space-cache.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> > index 74aa552f4793..c813378ebf08 100644
> > --- a/fs/btrfs/free-space-cache.c
> > +++ b/fs/btrfs/free-space-cache.c
> > @@ -3544,13 +3544,12 @@ int btrfs_write_out_ino_cache(struct btrfs_root *root,
> >  	struct btrfs_fs_info *fs_info = root->fs_info;
> >  	struct btrfs_free_space_ctl *ctl = root->free_ino_ctl;
> >  	int ret;
> > -	struct btrfs_io_ctl io_ctl;
> > +	struct btrfs_io_ctl io_ctl = {0};
> 
> Doesn't this zero the bytes unconditionally? The memset below happens
> only when the inode cache is on.

Yes, but does it matter. I assumed assignment is faster than memset, but
a google search says gcc optimization is smart enough now. So, this patch
seems irrelevant.

Patch
diff mbox series

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 74aa552f4793..c813378ebf08 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -3544,13 +3544,12 @@  int btrfs_write_out_ino_cache(struct btrfs_root *root,
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_free_space_ctl *ctl = root->free_ino_ctl;
 	int ret;
-	struct btrfs_io_ctl io_ctl;
+	struct btrfs_io_ctl io_ctl = {0};
 	bool release_metadata = true;
 
 	if (!btrfs_test_opt(fs_info, INODE_MAP_CACHE))
 		return 0;
 
-	memset(&io_ctl, 0, sizeof(io_ctl));
 	ret = __btrfs_write_out_cache(root, inode, ctl, NULL, &io_ctl, trans);
 	if (!ret) {
 		/*