Re: [PATCH] ext4: Optimize ext4 DIO overwrites

[Jan Kara <jack@suse.cz>]

On Thu 19-12-19 19:23:28, Ritesh Harjani wrote:
> On 12/18/19 11:14 PM, Jan Kara wrote:
> > Currently we start transaction for mapping every extent for writing
> > using direct IO. This is unnecessary when we know we are overwriting
> > already allocated blocks and the overhead of starting a transaction can
> > be significant especially for multithreaded workloads doing small writes.
> > Use iomap operations that avoid starting a transaction for direct IO
> > overwrites.
> > 
> > This improves throughput of 4k random writes - fio jobfile:
> > [global]
> > rw=randrw
> > norandommap=1
> > invalidate=0
> > bs=4k
> > numjobs=16
> > time_based=1
> > ramp_time=30
> > runtime=120
> > group_reporting=1
> > ioengine=psync
> > direct=1
> > size=16G
> > filename=file1.0.0:file1.0.1:file1.0.2:file1.0.3:file1.0.4:file1.0.5:file1.0.6:file1.0.7:file1.0.8:file1.0.9:file1.0.10:file1.0.11:file1.0.12:file1.0.13:file1.0.14:file1.0.15:file1.0.16:file1.0.17:file1.0.18:file1.0.19:file1.0.20:file1.0.21:file1.0.22:file1.0.23:file1.0.24:file1.0.25:file1.0.26:file1.0.27:file1.0.28:file1.0.29:file1.0.30:file1.0.31
> > file_service_type=random
> > nrfiles=32
> > 
> > from 3018MB/s to 4059MB/s in my test VM running test against simulated
> > pmem device (note that before iomap conversion, this workload was able
> > to achieve 3708MB/s because old direct IO path avoided transaction start
> > for overwrites as well). For dax, the win is even larger improving
> > throughput from 3042MB/s to 4311MB/s.
> 
> However for dax via ext4_dax_write_iter() path, we still need a way to
> detect if it's overwrite and that path can be optimized too right?
> I see, that this path could use both `shared inode locking` and
> `no journal transaction` optimizations in case of overwrites. Correct?

I don't think we can really afford the shared locking in
ext4_dax_write_iter() as POSIX requires overlapping writes to be
serialized. But we could still optimize-away the transaction starts.

> > Reported-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> This was one of the next AI I too wanted to do. I guess since everyone
> loves performance improvements. :)
> 
> No problem with current patch. Looks good. Gave it a run too on my
> system.
> 
> Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>

Thanks!

> However depending on which patch lands first one may need a
> re-basing. Will conflict with this-
> https://marc.info/?l=linux-ext4&m=157613016931238&w=2

Yes, but the conflict is minor and trivial to resolve.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR