Re: Discussion: is it time to remove dioread_nolock?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/8/20 11:12 PM, Theodore Y. Ts'o wrote:
On Wed, Jan 08, 2020 at 04:15:13PM +0530, Ritesh Harjani wrote:
Hello Ted/Jan,

On 1/7/20 10:52 PM, Jan Kara wrote:
On Tue 07-01-20 12:11:09, Theodore Y. Ts'o wrote:
Hmm..... There's actually an even more radical option we could use,
given that Ritesh has made dioread_nolock work on block sizes < page
size.  We could make dioread_nolock the default, until we can revamp
ext4_writepages() to write the data blocks first....

Agreed. I guess it should be a straight forward change to make.
It should be just removing test_opt(inode->i_sb, DIOREAD_NOLOCK) condition
from ext4_should_dioread_nolock().

Actually, it's simpler than that.  In fs/ext4/super.c, around line
3730, after the comment:

	/* Set defaults before we parse the mount options */

Just add:

      set_opt(sb, DIOREAD_NOLOCK);

Yes, silly me.



This will allow system administrators to revert back to the original
method using the someone confusingly named mount option,
"dioread_lock".  (Maybe we can add a alias for that mount option so
it's less confusing).

Yes, that's a good point. And I'm not opposed to that if it makes the life
simpler. But I'd like to see some performance numbers showing how much is
writeback using unwritten extents slower so that we don't introduce too big
regression with this...


Yes, let me try to get some performance numbers with dioread_nolock as
the default option for buffered write on my setup.

I started running some performance runs last night, and the
interesting thing that I found was that fs_mark actually *improved*
with dioread_nolock (with fsync enabled).  That may be an example of
where fixing the commit latency caused by writeback can actually show
up in a measurable way with benchmarks.

Dbench was slightly impacted; I didn't see any real differences with
compilebench or postmark.  dioread_nolock did improve fio with
sequential reads; which is interesting, since I would have expected

IIUC, this Seq. read numbers are with --direct=1 & bs=2MB & ioengine=libaio, correct?
So essentially it will do a DIO AIO sequential read.

Yes, it *does shows* a big delta in the numbers. I also noticed a higher
deviation between the two runs with dioread_nolock.


with the inode_lock improvements, there shouldn't have been any
difference.  So that may be a bit of wierdness that we should try to
understand.

So inode_lock patches gives improvement in mixed read/write workload
where inode exclusive locking was causing the bottleneck earlier.


In this run, was encryption or fsverity enabled?
If yes then in that case I see that ext4_dio_supported() will return
false and it will fallback to bufferedRead.
Though with that also can't explain the delta with only enabling
dioread_nolock.



See the attached tar file; open ext4-modes/index.html in a browser to
see the pretty graphs.  The raw numbers are in ext4/composite.xml.

The graphs and overview looks really good. I will also check about PTS
sometime. Will be good to capture such reports.


ritesh




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux