On Wed, Apr 18, 2012 at 08:09:02AM -0700, Andreas Dilger wrote:

Looking at these performance numbers again, it would seem better if ext4 _was_ zero filling the whole file and converting the whole thing to initialized extents instead of leaving so many uninitialized extents behind.

The file size is 256MB, and the disk would have to be doing only 3.5MB/s for linear streaming writes to match the performance that you report, so a modern disk doing 50MB/s should be able to zero the whole file in 5s.

It seems the threshold for zeroing uninitialized extents is incorrect. EXT4_EXT_ZERO_LEN is only 7 blocks (28kB normally), but typical disks can write 64kB as easily as 4kB, so it would be interesting to change EXT4_EXT_ZERO_LEN to 16 and re-run your test. 

If that solves this particular test case, it wont necessarily the general case, but is still a useful fix.  If you submit a patch for this, please change this code to compare against 64kB instead of a block count, and also to take s_raid_stride into account if set, like:

        ext_zero_len = max(EXT4_EXT_ZERO_LEN * 1024 >> inode->i_blkbits,
                           EXT4_SB(inode->i_sb)->s_es->s_raid_stride);

This would write up to 64kB, or a full RAID stripe (since it already needs to seek that spindle), whichever is larger.  It isn't perfect, since it should really align the zero-out to the RAID stripe to avoid seeking two spindles, but it is a starting point. 

Hi Andreas,

I set EXT4_EXT_ZERO_LEN to 16 and run the same benchmark again.  the result
is the same as before.

I notice this commit (3977c965) and it set EXT4_EXT_ZERO_LEN to 7.  But
in commit log, it doesn't describe why this value is set to 7.  As you
said, I believe that the disk writes 64K as easily as as 4k in modern
disk.  So maybe we can consider to set it to 16 or RAID stripe. :)

Regards,
Zheng