Re: [RFCv5 5/5] iomap: Add per-block dirty state tracking to improve performance

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Pankaj Raghav <p.raghav@samsung.com> writes:

>> @@ -1666,7 +1766,7 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
>>  		struct writeback_control *wbc, struct inode *inode,
>>  		struct folio *folio, u64 end_pos)
>>  {
>> -	struct iomap_page *iop = iop_alloc(inode, folio, 0);
>> +	struct iomap_page *iop = iop_alloc(inode, folio, 0, true);
>>  	struct iomap_ioend *ioend, *next;
>>  	unsigned len = i_blocksize(inode);
>>  	unsigned nblocks = i_blocks_per_folio(inode, folio);
>> @@ -1682,7 +1782,7 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
>>  	 * invalid, grab a new one.
>>  	 */
>>  	for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) {
>> -		if (iop && !iop_test_block_uptodate(folio, i))
>> +		if (iop && !iop_test_block_dirty(folio, i))
>
> Shouldn't this be if(iop && iop_test_block_dirty(folio, i))?
>
> Before we were skipping if the blocks were not uptodate but now we are
> skipping if the blocks are not dirty (which means they are uptodate)?
>
> I am new to iomap but let me know if I am missing something here.
>

We set the per-block dirty status in ->write_begin. The check above happens in the
writeback path when we are about to write the dirty data to the disk.
What the check is doing is that, it checks if the block state is not dirty
then skip it which means the block was not written to in the ->write_begin().
Also the block with dirty status always means that the block is uptodate
anyways.

Hope it helps!

-ritesh