Received-SPF: pass (google.com: domain of linux-ext4+bounces-2291-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <eb812b45-e28d-430d-a7a3-5b73edcc8057@suse.com>
Date: Sat, 4 May 2024 09:36:33 +0930
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file
 data extents
To: Anand Jain <anand.jain@oracle.com>, linux-btrfs@vger.kernel.org,
 linux-ext4@vger.kernel.org
References: <cover.1714722726.git.anand.jain@oracle.com>
 <6d2a19ced4551bfcf2a5d841921af7f84c4ea950.1714722726.git.anand.jain@oracle.com>
 <48787c70-1abf-433e-ad3f-9e212237f9a5@suse.com>
 <8b5107fa-bcce-46dd-950b-775695d872e6@oracle.com>
 <459ec128-eddd-4575-ab28-788f340a6a7c@suse.com>
 <7a33ff31-e570-4775-b82f-1c6413656699@oracle.com>
Content-Language: en-US
From: Qu Wenruo <wqu@suse.com>
Autocrypt: addr=wqu@suse.com; keydata=
 xsBNBFnVga8BCACyhFP3ExcTIuB73jDIBA/vSoYcTyysFQzPvez64TUSCv1SgXEByR7fju3o
 8RfaWuHCnkkea5luuTZMqfgTXrun2dqNVYDNOV6RIVrc4YuG20yhC1epnV55fJCThqij0MRL
 1NxPKXIlEdHvN0Kov3CtWA+R1iNN0RCeVun7rmOrrjBK573aWC5sgP7YsBOLK79H3tmUtz6b
 9Imuj0ZyEsa76Xg9PX9Hn2myKj1hfWGS+5og9Va4hrwQC8ipjXik6NKR5GDV+hOZkktU81G5
 gkQtGB9jOAYRs86QG/b7PtIlbd3+pppT0gaS+wvwMs8cuNG+Pu6KO1oC4jgdseFLu7NpABEB
 AAHNGFF1IFdlbnJ1byA8d3F1QHN1c2UuY29tPsLAlAQTAQgAPgIbAwULCQgHAgYVCAkKCwIE
 FgIDAQIeAQIXgBYhBC3fcuWlpVuonapC4cI9kfOhJf6oBQJjTSJVBQkNOgemAAoJEMI9kfOh
 Jf6oapEH/3r/xcalNXMvyRODoprkDraOPbCnULLPNwwp4wLP0/nKXvAlhvRbDpyx1+Ht/3gW
 p+Klw+S9zBQemxu+6v5nX8zny8l7Q6nAM5InkLaD7U5OLRgJ0O1MNr/UTODIEVx3uzD2X6MR
 ECMigQxu9c3XKSELXVjTJYgRrEo8o2qb7xoInk4mlleji2rRrqBh1rS0pEexImWphJi+Xgp3
 dxRGHsNGEbJ5+9yK9Nc5r67EYG4bwm+06yVT8aQS58ZI22C/UeJpPwcsYrdABcisd7dddj4Q
 RhWiO4Iy5MTGUD7PdfIkQ40iRcQzVEL1BeidP8v8C4LVGmk4vD1wF6xTjQRKfXHOwE0EWdWB
 rwEIAKpT62HgSzL9zwGe+WIUCMB+nOEjXAfvoUPUwk+YCEDcOdfkkM5FyBoJs8TCEuPXGXBO
 Cl5P5B8OYYnkHkGWutAVlUTV8KESOIm/KJIA7jJA+Ss9VhMjtePfgWexw+P8itFRSRrrwyUf
 E+0WcAevblUi45LjWWZgpg3A80tHP0iToOZ5MbdYk7YFBE29cDSleskfV80ZKxFv6koQocq0
 vXzTfHvXNDELAuH7Ms/WJcdUzmPyBf3Oq6mKBBH8J6XZc9LjjNZwNbyvsHSrV5bgmu/THX2n
 g/3be+iqf6OggCiy3I1NSMJ5KtR0q2H2Nx2Vqb1fYPOID8McMV9Ll6rh8S8AEQEAAcLAfAQY
 AQgAJgIbDBYhBC3fcuWlpVuonapC4cI9kfOhJf6oBQJjTSJuBQkNOge/AAoJEMI9kfOhJf6o
 rq8H/3LJmWxL6KO2y/BgOMYDZaFWE3TtdrlIEG8YIDJzIYbNIyQ4lw61RR+0P4APKstsu5VJ
 9E3WR7vfxSiOmHCRIWPi32xwbkD5TwaA5m2uVg6xjb5wbdHm+OhdSBcw/fsg19aHQpsmh1/Q
 bjzGi56yfTxxt9R2WmFIxe6MIDzLlNw3JG42/ark2LOXywqFRnOHgFqxygoMKEG7OcGy5wJM
 AavA+Abj+6XoedYTwOKkwq+RX2hvXElLZbhYlE+npB1WsFYn1wJ22lHoZsuJCLba5lehI+//
 ShSsZT5Tlfgi92e9P7y+I/OzMvnBezAll+p/Ly2YczznKM5tV0gboCWeusM=
In-Reply-To: <7a33ff31-e570-4775-b82f-1c6413656699@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


在 2024/5/4 08:57, Anand Jain 写道:
> 
> 
> On 5/4/24 06:23, Qu Wenruo wrote:
>>
>>
>> 在 2024/5/3 21:55, Anand Jain 写道:
>> [...]
>>>>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>>>>> *prealloc)
>>>>
>>>> This function is called for every file extent we're going to create.
>>>> I'm not a big fan of doing so many lookup.
>>>>
>>>> My question is, is this the only way to determine the flag of the 
>>>> data extent?
>>>>
>>>> My instinct says there should be a straight forward way to determine 
>>>> if a file extent is preallocated or not, just like what we do in our 
>>>> file extent items.
>>>
>>>
>>>> Thus during the ext2fs_block_iterate2(), there should be some way to 
>>>> tell if a block is preallocated or not.
>>>
>>> Unfortunately, the callback doesn't provide the extent flags. Unless, 
>>> I miss something?
>>
>> You're right, the iterator interface does not provide any extra info.
>>
>> And I also checked the kernel implementation, they have extra 
>> ext4_map_blocks() to do the resolve, and then ext4_es_lookup_extent() 
>> to determine if it's unwritten.
>>
>> So I'm afraid we have to go this solution.
>>
>>
>> Meanwhile related to the implementation, can we put the prealloc flat 
>> into blk_iterate_data?
>> So that we can handle different fses' preallocated extents in a more 
>> common way.
>>
> 
> I initially thought so, but is blk_iterate_data::num_blocks always
> equal to extent::e_len in all file data extent situations mixed
> with hole and unwritten combinations? If not, then the flag might
> not be appropriate there, as it doesn't apply to the entirety of
> blk_iterate_data::num_blocks.

I do not think we need @num_blocks to match extent_len.

We're already doing some kind of merge inside block_iterate_proc(), and 
if we find previous extent flag doesn't match the current one, we just 
need to submit the previous one.

Although I also believe we need some better abstraction for the common code.
The current one doesn't explain everything well for things parameters 
like disk_block/file_block.

Thanks,
Qu


> 
> Thanks, Anand
> 
>> Thanks,
>> Qu