linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Jan Kara <jack@suse.cz>
Cc: Brian Foster <bfoster@redhat.com>,
	linux-fsdevel@vger.kernel.org, dchinner@redhat.com,
	jbacik@fb.com
Subject: Re: [PATCH v6 1/2] sb: add a new writeback list for sync
Date: Thu, 21 Jan 2016 07:11:59 +1100	[thread overview]
Message-ID: <20160120201159.GW6033@dastard> (raw)
In-Reply-To: <20160120132626.GE10810@quack.suse.cz>

On Wed, Jan 20, 2016 at 02:26:26PM +0100, Jan Kara wrote:
> On Tue 19-01-16 12:59:12, Brian Foster wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > wait_sb_inodes() currently does a walk of all inodes in the
> > filesystem to find dirty one to wait on during sync. This is highly
> > inefficient and wastes a lot of CPU when there are lots of clean
> > cached inodes that we don't need to wait on.
> > 
> > To avoid this "all inode" walk, we need to track inodes that are
> > currently under writeback that we need to wait for. We do this by
> > adding inodes to a writeback list on the sb when the mapping is
> > first tagged as having pages under writeback. wait_sb_inodes() can
> > then walk this list of "inodes under IO" and wait specifically just
> > for the inodes that the current sync(2) needs to wait for.
> > 
> > Define a couple helpers to add/remove an inode from the writeback
> > list and call them when the overall mapping is tagged for or cleared
> > from writeback. Update wait_sb_inodes() to walk only the inodes
> > under writeback due to the sync.
> 
> The patch looks good.  Just one comment: This grows struct inode by two
> longs. Such a growth should be justified by measuring the improvements. So
> can you measure some numbers showing how much the patch helped? I think it
> would be interesting to see:
> 
> a) How much sync(2) speed has improved if there's not much to wait for.

Depends on the size of the inode cache when sync is run.  If it's
empty it's not noticable. When you have tens of millions of cached,
clean inodes the inode list traversal can takes tens of seconds.
This is the sort of problem Josef reported that FB were having...

> b) See whether parallel heavy stat(2) load which is rotating lots of inodes
> in inode cache sees some improvement when it doesn't have to contend with
> sync(2) on s_inode_list_lock. I believe Dave Chinner had some loads where
> the contention on s_inode_list_lock due to sync and rotation of inodes was
> pretty heavy.

Just my usual fsmark workloads - they have parallel find and
parallel ls -lR traversals over the created fileset. Even just
running sync during creation (because there are millions of cached
inodes, and ~250,000 inodes being instiated and reclaimed every
second) causes lock contention problems....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2016-01-20 20:12 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-19 17:59 [PATCH v6 0/2] improve sync efficiency with sb inode wb list Brian Foster
2016-01-19 17:59 ` [PATCH v6 1/2] sb: add a new writeback list for sync Brian Foster
2016-01-20 13:26   ` Jan Kara
2016-01-20 20:11     ` Dave Chinner [this message]
2016-01-21 15:22       ` Brian Foster
2016-01-21 16:34         ` Jan Kara
2016-01-21 17:13           ` Brian Foster
2016-01-21 18:08             ` Josef Bacik
2016-01-19 17:59 ` [PATCH v6 2/2] wb: inode writeback list tracking tracepoints Brian Foster
2016-01-20 13:14   ` Jan Kara

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=20160120201159.GW6033@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=dchinner@redhat.com \
    --cc=jack@suse.cz \
    --cc=jbacik@fb.com \
    --cc=linux-fsdevel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).