All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <jbacik@redhat.com>
To: Andreas Dilger <adilger@sun.com>
Cc: Josef Bacik <jbacik@redhat.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org, rwheeler@redhat.com
Subject: Re: [PATCH] improve jbd fsync batching
Date: Tue, 28 Oct 2008 17:33:10 -0400	[thread overview]
Message-ID: <20081028213310.GC21600@unused.rdu.redhat.com> (raw)
In-Reply-To: <20081028213805.GC3184@webber.adilger.int>

On Tue, Oct 28, 2008 at 03:38:05PM -0600, Andreas Dilger wrote:
> On Oct 28, 2008  16:16 -0400, Josef Bacik wrote:
> > I also have a min() check in there to make sure we don't sleep longer than
> > a jiffie in case our storage is super slow, this was requested by Andrew.
> 
> Is there a particular reason why 1 jiffie is considered the "right amount"
> of time to sleep, given this is a kernel config parameter and has nothing
> to do with the storage?  Considering a seek time in the range of ~10ms
> this would only be right for HZ=100 and the wait would otherwise be too
> short to maximize batching within a single transaction.
>

I wouldn't say "right amount", more of "traditional amount".  If you have super
slow storage this patch will not result in you waiting any longer than you did
originally, which I think is what the concern was, that we not wait a super long
time just because the disk is slow.
 
> > type	threads		with patch	without patch
> > sata	2		24.6		26.3
> > sata	4		49.2		48.1
> > sata	8		70.1		67.0
> > sata	16		104.0		94.1
> > sata	32		153.6		142.7
> 
> In the previous patch where this wasn't limited it had better performance
> even for the 2 thread case.  With the current 1-jiffie wait it likely
> isn't long enough to batch every pair of operations and every other
> operation waits an extra amount before giving up too soon.  Previous patch:
> 
> type    threads       patch     unpatched
> sata    2              34.6     26.2
> sata    4              58.0     48.0
> sata    8              75.2     70.4
> sata    16            101.1     89.6
> 
> I'd recommend changing the patch to have a maximum sleep time that has a
> fixed maximum number of milliseconds (15ms should be enough for even very
> old disks).
> 

This stat gathering process has been very unscientific :), I just ran once and
took that number.  Sometimes the patched version would come out on top,
sometimes it wouldn't.  If I were to do this the way my stat teacher taught me
I'm sure the patched/unpatched versions would come out to be relatively the same
in the 2 thread case.

> 
> That said, this would be a minor enhancement and should NOT be considered
> a reason to delay this patch's inclusion into -mm or the ext4 tree.
> 
> PS - it should really go into jbd2 also
>

Yes I will be doing a jbd2 version of this patch provided there are no issues
with this patch.  Thanks much for the comments,

Josef 

  reply	other threads:[~2008-10-28 21:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-28 20:16 [PATCH] improve jbd fsync batching Josef Bacik
2008-10-28 21:38 ` Andreas Dilger
2008-10-28 21:33   ` Josef Bacik [this message]
2008-10-28 21:44   ` Arjan van de Ven
2008-10-28 21:56     ` Ric Wheeler
2008-11-03 20:27 ` Andrew Morton
2008-11-03 20:24   ` Josef Bacik
2008-11-03 20:55     ` Andrew Morton
2008-11-04 15:41       ` Josef Bacik
2008-11-03 22:13     ` Theodore Tso
2008-11-04  5:24   ` Andreas Dilger
2008-11-04  9:12     ` Andrew Morton

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=20081028213310.GC21600@unused.rdu.redhat.com \
    --to=jbacik@redhat.com \
    --cc=adilger@sun.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rwheeler@redhat.com \
    /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.