Re: [PATCH 3/8] xfs: initialise xfssync work before running quotachecks

From: Mark Tinguely <tinguely@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Ben Myers <bpm@sgi.com>, xfs@oss.sgi.com
Subject: Re: [PATCH 3/8] xfs: initialise xfssync work before running quotachecks
Date: Mon, 26 Mar 2012 10:10:50 -0500	[thread overview]
Message-ID: <4F7086FA.9010003@sgi.com> (raw)
In-Reply-To: <20120325232253.GJ5091@dastard>

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().

I mentioned the lock and the 2 or 3 unbuffered I/O because they are the 
potential items that can take some time between starting the sync 
workers and intializing the m_perag_tree radix tree.

>> >  I was also looking at the xfs_perag_t being allocated in mountfs()
>> >  and being deallocated in umountfs(), but it turns out that is not
>> >  important, xfs_perag_get() will return NULL if these have not been
>> >  allocated yet or have been removed for the required ag.
> Correct. The other side of that is that if xfssyncd is doing some
> form of inode cache iteration, it will simply not find any perag
> structures to scani and hence won't cause problems. Same with the
> reclaim worker.
>
> Were there any other issues?
>
> Cheers,
>
> Dave.
> -- Dave Chinner david@fromorbit.com

Thanks,

--Mark Tinguely

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