Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/4] ext4: remove dead GET_BLOCKS_ZERO code
To:     Eric Whitney <enwlinux@gmail.com>, linux-ext4@vger.kernel.org
Cc:     tytso@mit.edu
References: <20200430185320.23001-1-enwlinux@gmail.com>
 <20200430185320.23001-2-enwlinux@gmail.com>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Date:   Fri, 1 May 2020 02:32:10 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20200430185320.23001-2-enwlinux@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <20200430210211.2A2A0AE04D@d06av26.portsmouth.uk.ibm.com>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk


On 5/1/20 12:23 AM, Eric Whitney wrote:
> There's no call to ext4_map_blocks() in the current ext4 code with a
> flags argument that combines EXT4_GET_BLOCKS_CONVERT and
> EXT4_GET_BLOCKS_ZERO.  Remove the code that corresponds to this case
> from ext4_ext_handle_unwritten_extents().
> 
> Signed-off-by: Eric Whitney <enwlinux@gmail.com>

As I see it. Yes, this flag was mainly added for DAX handling at two 
places but mostly with below purpose.

Purpose:- Since DAX earlier using PRE_IO flag and then to convert
unwritten to written, it added this extra functionality to zero out.
Since ext4_map_blocks already implements the unwritten to written
functionality, so PRE_IO along with below combination of flags was
removed from DAX path.

Now none of that DAX code path uses below code anyways. So your patch
justifies killing below code snip.


Feel free to add:
Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>

> ---
>   fs/ext4/extents.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index f2b577b315a0..59a90492b9dd 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3826,14 +3826,6 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
>   	}
>   	/* IO end_io complete, convert the filled extent to written */
>   	if (flags & EXT4_GET_BLOCKS_CONVERT) {
> -		if (flags & EXT4_GET_BLOCKS_ZERO) {
> -			if (allocated > map->m_len)
> -				allocated = map->m_len;
> -			err = ext4_issue_zeroout(inode, map->m_lblk, newblock,
> -						 allocated);
> -			if (err < 0)
> -				goto out2;
> -		}
>   		ret = ext4_convert_unwritten_extents_endio(handle, inode, map,
>   							   ppath);
>   		if (ret >= 0)
>