Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCHv3 4/6] ext4: Make ext4_ind_map_blocks work with fiemap
To:     Jan Kara <jack@suse.cz>
Cc:     tytso@mit.edu, linux-ext4@vger.kernel.org,
        adilger.kernel@dilger.ca, linux-fsdevel@vger.kernel.org,
        darrick.wong@oracle.com, hch@infradead.org, cmaiolino@redhat.com
References: <cover.1582702693.git.riteshh@linux.ibm.com>
 <56fc8d3802c578d27d49270600946a0737cef119.1582702694.git.riteshh@linux.ibm.com>
 <20200226123940.GP10728@quack2.suse.cz>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Date:   Wed, 26 Feb 2020 18:17:21 +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: <20200226123940.GP10728@quack2.suse.cz>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <20200226124722.278C852050@d06av21.portsmouth.uk.ibm.com>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk


On 2/26/20 6:09 PM, Jan Kara wrote:
> On Wed 26-02-20 15:27:06, Ritesh Harjani wrote:
>> For indirect block mapping if the i_block > max supported block in inode
>> then ext4_ind_map_blocks may return a -EIO error. But in case of fiemap
>> this could be a valid query to ext4_map_blocks.
>> So in case if !create then return 0. This also makes ext4_warning to
>> ext4_debug in ext4_block_to_path() for the same reason.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> 
> Hmm, won't it be cleaner to just handle this in ext4_iomap_begin_report()?
> We do trim map.m_len there anyway so it is only logical to trim it to
> proper value supported by the inode on-disk format... BTW, note we have
> sbi->s_bitmap_maxbytes value already computed in the superblock...

hmm. Yes, thanks for the pointers. Let me check this again.

-ritesh


> 
> 								Honza
> 
>> ---
>>   fs/ext4/indirect.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
>> index 3a4ab70fe9e0..e1ab495dd900 100644
>> --- a/fs/ext4/indirect.c
>> +++ b/fs/ext4/indirect.c
>> @@ -102,7 +102,11 @@ static int ext4_block_to_path(struct inode *inode,
>>   		offsets[n++] = i_block & (ptrs - 1);
>>   		final = ptrs;
>>   	} else {
>> -		ext4_warning(inode->i_sb, "block %lu > max in inode %lu",
>> +		/*
>> +		 * It's not yet an error to just query beyond max
>> +		 * block in inode. Fiemap callers may do so.
>> +		 */
>> +		ext4_debug("block %lu > max in inode %lu",
>>   			     i_block + direct_blocks +
>>   			     indirect_blocks + double_blocks, inode->i_ino);
>>   	}
>> @@ -537,8 +541,11 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
>>   	depth = ext4_block_to_path(inode, map->m_lblk, offsets,
>>   				   &blocks_to_boundary);
>>   
>> -	if (depth == 0)
>> +	if (depth == 0) {
>> +		if (!(flags & EXT4_GET_BLOCKS_CREATE))
>> +			err = 0;
>>   		goto out;
>> +	}
>>   
>>   	partial = ext4_get_branch(inode, depth, offsets, chain, &err);
>>   
>> -- 
>> 2.21.0
>>