From: Boaz Harrosh <bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
To: Josef Bacik <josef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: <linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
<adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>,
<tytso-3s7WtUTddSA@public.gmane.org>,
<jack-AlSwsSmVLrQ@public.gmane.org>
Subject: Re: [PATCH] ext4: fix how i_version is modified and turn it on by default V2
Date: Tue, 15 May 2012 21:17:16 +0300 [thread overview]
Message-ID: <4FB29DAC.3020801@panasas.com> (raw)
In-Reply-To: <20120515175308.GB1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On 05/15/2012 08:53 PM, Josef Bacik wrote:
> On Tue, May 15, 2012 at 10:33:16AM -0400, Josef Bacik wrote:
>> This makes MS_I_VERSION be turned on by default. Ext4 had been
>> unconditionally doing i_version++ in a few cases anway so the mount option
>> was kind of silly. This patch also removes the update in mark_inode_dirty
>> and makes all of the cases where we update ctime also do inode_inc_ב.
>> file_update_time takes care of the write case and all the places where we
>> update iversion are protected by the i_mutex so there should be no extra
>> i_lock overhead in the normal non-exported fs case. Thanks,
>>
>
> Ok did some basic benchmarking with dd, I ran
>
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1 count=10485760
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1M count=1000
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1M count=5000
>
> 3 times with the patch and without the patch. With the worst case scenario
> there is about a 40% longer run time, going from on average 12 seconds to 17
> seconds.
do you mean that the "with the patch" is 40% slower then "without the patch"
in the same "bs=1 count=10485760 test" ?
Then count me clueless. Do you understand this difference?
The way I read your patch the inode is copied and written to disk exactly the
same number of times, as before. Only that now i_version is also updated
together with ctime and/or mtime. What is the fundamental difference then?
Is it just that i_version++ in-memory operation?
> With the other two runs they are the same runtime with the 1 megabyte
> blocks. So the question is, do we care about this worst case since any sane
> application developer isn't going to do writes that small? Thanks,
> Josef
Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Boaz Harrosh <bharrosh@panasas.com>
To: Josef Bacik <josef@redhat.com>
Cc: <linux-ext4@vger.kernel.org>, <linux-nfs@vger.kernel.org>,
<bfields@fieldses.org>, <adilger@dilger.ca>, <tytso@mit.edu>,
<jack@suse.cz>
Subject: Re: [PATCH] ext4: fix how i_version is modified and turn it on by default V2
Date: Tue, 15 May 2012 21:17:16 +0300 [thread overview]
Message-ID: <4FB29DAC.3020801@panasas.com> (raw)
In-Reply-To: <20120515175308.GB1907@localhost.localdomain>
On 05/15/2012 08:53 PM, Josef Bacik wrote:
> On Tue, May 15, 2012 at 10:33:16AM -0400, Josef Bacik wrote:
>> This makes MS_I_VERSION be turned on by default. Ext4 had been
>> unconditionally doing i_version++ in a few cases anway so the mount option
>> was kind of silly. This patch also removes the update in mark_inode_dirty
>> and makes all of the cases where we update ctime also do inode_inc_ב.
>> file_update_time takes care of the write case and all the places where we
>> update iversion are protected by the i_mutex so there should be no extra
>> i_lock overhead in the normal non-exported fs case. Thanks,
>>
>
> Ok did some basic benchmarking with dd, I ran
>
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1 count=10485760
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1M count=1000
> dd if=/dev/zero of=/mnt/btrfs-test/file bs=1M count=5000
>
> 3 times with the patch and without the patch. With the worst case scenario
> there is about a 40% longer run time, going from on average 12 seconds to 17
> seconds.
do you mean that the "with the patch" is 40% slower then "without the patch"
in the same "bs=1 count=10485760 test" ?
Then count me clueless. Do you understand this difference?
The way I read your patch the inode is copied and written to disk exactly the
same number of times, as before. Only that now i_version is also updated
together with ctime and/or mtime. What is the fundamental difference then?
Is it just that i_version++ in-memory operation?
> With the other two runs they are the same runtime with the 1 megabyte
> blocks. So the question is, do we care about this worst case since any sane
> application developer isn't going to do writes that small? Thanks,
> Josef
Thanks
Boaz
next prev parent reply other threads:[~2012-05-15 18:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-15 14:33 [PATCH] ext4: fix how i_version is modified and turn it on by default V2 Josef Bacik
2012-05-15 14:33 ` Josef Bacik
2012-05-15 15:18 ` Jan Kara
2012-05-15 17:53 ` Josef Bacik
[not found] ` <20120515175308.GB1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 18:17 ` Boaz Harrosh [this message]
2012-05-15 18:17 ` Boaz Harrosh
2012-05-15 18:29 ` Josef Bacik
2012-05-15 18:29 ` Josef Bacik
2012-05-15 19:55 ` Andreas Dilger
[not found] ` <AB91C817-DFEE-415F-8769-78831D72C6B7-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
2012-05-15 20:05 ` Josef Bacik
2012-05-15 20:05 ` Josef Bacik
[not found] ` <20120515200533.GD1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 21:00 ` J. Bruce Fields
2012-05-15 21:00 ` J. Bruce Fields
[not found] ` <20120515210029.GA11932-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2012-05-15 21:08 ` Josef Bacik
2012-05-15 21:08 ` Josef Bacik
2012-05-15 22:19 ` Andreas Dilger
2012-05-15 22:19 ` Andreas Dilger
[not found] ` <20120515182914.GC1907-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2012-05-15 20:24 ` Boaz Harrosh
2012-05-15 20:24 ` Boaz Harrosh
2012-05-16 1:36 ` Ted Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FB29DAC.3020801@panasas.com \
--to=bharrosh-c4p08nqkorlbdgjk7y7tuq@public.gmane.org \
--cc=adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org \
--cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
--cc=jack-AlSwsSmVLrQ@public.gmane.org \
--cc=josef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=tytso-3s7WtUTddSA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.