All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Mark Tinguely <tinguely@sgi.com>
Cc: Ben Myers <bpm@sgi.com>, xfs@oss.sgi.com
Subject: Re: [PATCH 3/8] xfs: initialise xfssync work before running quotachecks
Date: Tue, 27 Mar 2012 08:57:09 +1100	[thread overview]
Message-ID: <20120326215709.GM5091@dastard> (raw)
In-Reply-To: <4F7086FA.9010003@sgi.com>

On Mon, Mar 26, 2012 at 10:10:50AM -0500, Mark Tinguely wrote:
> On 03/25/12 18:22, Dave Chinner wrote:
> >On Fri, Mar 23, 2012 at 08:34:31AM -0500, Mark Tinguely wrote:
> >>>  On 03/22/12 16:07, Dave Chinner wrote:
> >>>>  >On Thu, Mar 22, 2012 at 10:15:48AM -0500, Ben Myers wrote:
> >>>>>  >>On Thu, Mar 22, 2012 at 04:15:08PM +1100, Dave Chinner wrote:
> >>>>>>  >>>From: Dave Chinner<dchinner@redhat.com>
> >>>>>>  >>>
> >>>>>>  >>>Because the mount process can run a quotacheck and consume lots of
> >>>>>>  >>>inodes, we need to be able to run periodic inode reclaim during the
> >>>>>>  >>>mount process. This will prevent running the system out of memory
> >>>>>>  >>>during quota checks.
> >>>>>>  >>>
> >>>>>>  >>>This essentially reverts 2bcf6e97, but that is safe to do now that
> >>>>>>  >>>the quota sync code that was causing problems during long quotacheck
> >>>>>>  >>>executions is now gone.
> >>>>>  >>
> >>>>>  >>Dave, I've held off on #s 3 and 4 because they appear to be racy.  Being
> >>>>  >
> >>>>  >What race?
> >>>>  >
> >>>>  >Cheers,
> >>>>  >
> >>>>  >Dave
> >>>
> >>>
> >>>  2 of the sync workers use iterators
> >>>    xfs_inode_ag_iterator()
> >>>     xfs_perag_get()
> >>>      radix_tree_lookup(&mp->m_perag_tree, agno)
> >>>
> >>>  The race I was worried about was in xfs_mount() to initialize the
> >>>  mp->m_perag_lock, and the radix tree initialization:
> >>>    INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC)).
> >>>
> >>>  There is a lock and 2 or 3 unbuffered I/O are performed in xfs_mountfs()
> >>>  before the mp->m_perag_tree is initialized.
> >Yes they are uncached IOs so do not utilise the cache that
> >requires the mp->m_perag_tree to be initialised.
> 
> The point I was trying to make is the sync workers use iterators.
> The race is to get the mp->m_perag_tree initialized before one of
> the sync workers tries to do a xfs_perag_get().

Firstly, xfs_sync_worker does not iterate AGs at all anymore - it
pushes the log and the AIL, and nothing else. So there is no
problems there.

Secondly xfs_flush_worker() is only triggered by ENOSPC, and that
can't happen until the filesystem is mounted and real work starts.

Finally, the reclaim worker does iterate the perag tree, but the
next patch in the series ensures that is started on demand, not from
xfs_syncd_init(). This ensures that iteration does not occur
until after the first inode is placed into reclaim, and that must
happen after the perag tree is initialised because otherwise we
can't read in inodes, let alone put them into a reclaim state....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2012-03-26 21:57 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-22  5:15 [PATH 0/8] xfs: outstanding patches for 3.4 merge window Dave Chinner
2012-03-22  5:15 ` [PATCH 1/8] xfs: Fix open flag handling in open_by_handle code Dave Chinner
2012-03-22  5:15 ` [PATCH 2/8] xfs: introduce an allocation workqueue Dave Chinner
2012-03-22  5:15 ` [PATCH 3/8] xfs: initialise xfssync work before running quotachecks Dave Chinner
2012-03-22 15:15   ` Ben Myers
2012-03-22 21:07     ` Dave Chinner
2012-03-23 13:34       ` Mark Tinguely
2012-03-25 23:22         ` Dave Chinner
2012-03-26 15:10           ` Mark Tinguely
2012-03-26 21:57             ` Dave Chinner [this message]
2012-03-28 19:40               ` Ben Myers
2012-03-29  0:29                 ` Dave Chinner
2012-03-29  6:30                   ` [PATCH v2] xfs: Ensure inode reclaim can run during quotacheck Dave Chinner
2012-03-28 17:38   ` [PATCH 3/8] xfs: initialise xfssync work before running quotachecks Ben Myers
2012-03-28 18:02     ` Christoph Hellwig
2012-03-28 18:43       ` Ben Myers
2012-03-22  5:15 ` [PATCH 4/8] xfs: remove MS_ACTIVE guard from inode reclaim work Dave Chinner
2012-03-22  5:15 ` [PATCH 5/8] xfs: don't cache inodes read through bulkstat Dave Chinner
2012-03-22  5:15 ` [PATCH 6/8] xfs: Account log unmount transaction correctly Dave Chinner
2012-03-24  8:27   ` Christoph Hellwig
2012-03-24 22:49     ` Dave Chinner
2012-03-26 22:28   ` Ben Myers
2012-03-22  5:15 ` [PATCH 7/8] xfs: fix fstrim offset calculations Dave Chinner
2012-03-24 13:36   ` Christoph Hellwig
2012-03-27 20:48   ` Ben Myers
2012-03-27 21:42     ` Dave Chinner
2012-03-22  5:15 ` [PATCH 8/8] xfs: add lots of attribute trace points Dave Chinner
2012-03-24 13:42   ` Christoph Hellwig
2012-03-27 21:18   ` Ben Myers
2012-03-27 21:45     ` Dave Chinner
2012-03-27 22:01       ` Ben Myers

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=20120326215709.GM5091@dastard \
    --to=david@fromorbit.com \
    --cc=bpm@sgi.com \
    --cc=tinguely@sgi.com \
    --cc=xfs@oss.sgi.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.