All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH v3] xfs: re-enable xfsaild idle mode and fix associated races
Date: Tue, 03 Jul 2012 09:13:57 -0400	[thread overview]
Message-ID: <4FF2F015.6090109@redhat.com> (raw)
In-Reply-To: <20120702235106.GU19223@dastard>

On 07/02/2012 07:51 PM, Dave Chinner wrote:
> On Mon, Jul 02, 2012 at 09:33:24AM -0400, Brian Foster wrote:
>> On 07/01/2012 08:07 PM, Dave Chinner wrote:
>>> On Thu, Jun 28, 2012 at 06:52:56AM -0400, Brian Foster wrote:
>>>> xfsaild idle mode logic currently leads to a couple hangs:
>>>>
>>>> 1.) If xfsaild is rescheduled in during an incremental scan
>>>>     (i.e., tout != 0) and the target has been updated since
>>>>     the previous run, we can hit the new target and go into
>>>>     idle mode with a still populated ail.
>>>> 2.) A wake up is only issued when the target is pushed forward.
>>>>     The wake up can race with xfsaild if it is currently in the
>>>>     process of entering idle mode, causing future wake up
>>>>     events to be lost.
>>>>
>>>> These hangs have been reproduced and verified as fixed by
>>>> running xfstests 273 in a loop on a slightly modified upstream
>>>> kernel. The kernel is modified to re-enable idle mode as
>>>> previously implemented (when count == 0) and with a revert of
>>>> commit 670ce93f, which includes performance improvements that
>>>> make this harder to reproduce.
>>>>
>>>> The solution, the algorithm for which has been outlined by
>>>> Dave Chinner, is to modify xfsaild to enter idle mode only when
>>>> the ail is empty and the push target has not been moved forward
>>>> since the last push.
>>>>
>>>> Signed-off-by: Brian Foster <bfoster@redhat.com>
>>>
>>> Looks OK to me, and hasn't caused any problems here.
>>>
>>> Final question - did you confirm with powertop that the xfsaild is
>>> no longer causing wakeups a minute or two after you stop writing to
>>> the filesystem? (I haven't yet)
>>>
>>
>> I hadn't tested with powertop, but I had some tracepoints hacked in
>> around the idle/wake cases to verify the thread was actually scheduling
>> out.
> 
> If you've added tracepoints that were useful for
> debugging/verification, then send that as a patch as well. If users
> have trouble then simply asking them for event traces is very easy
> to do and gives us much better insight into what is happening....
> 
> You can't have enough tracepoints when things are going wrong ;)
> 

Ok, duly noted. What I have right now is scattered about a few branches
and not immediately presentable. When I get some time I'll fix them up
and post. If I remember correctly, I had covered: xfsaild end (count,
skip, target, etc.), xfsaild idle, xa_target update (xfs_ail_push()) and
xfsaild wake (which might be extraneous at this point).

Brian

>> FWIW, I just gave powertop a quick test and it appears to work as
>> expected...
>>
>> With current upstream on my rhel6.3 VM, I see the following after
>> running a 'touch /mnt/file;sync' and letting the fs idle for a bit:
>>
>>    0.5% ( 19.9)      xfsaild/vdb1 : xfsaild (process_timeout)
>>
>> and this drops off completely with the patch applied. Thanks for the tip.
> 
> Great, then it is working exactly as expected.
> 
> Cheers,
> 
> Dave.
> 


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

  reply	other threads:[~2012-07-03 13:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-28 10:52 [PATCH v3] xfs: re-enable xfsaild idle mode and fix associated races Brian Foster
2012-07-02  0:07 ` Dave Chinner
2012-07-02 13:33   ` Brian Foster
2012-07-02 23:51     ` Dave Chinner
2012-07-03 13:13       ` Brian Foster [this message]
2012-07-03 16:07         ` Christoph Hellwig
2012-07-02  7:05 ` Christoph Hellwig
2012-07-02  8:29   ` Dave Chinner
2012-07-02 13:51     ` Brian Foster
2012-07-17  7:00       ` Christoph Hellwig
2012-07-24 13:22         ` Christoph Hellwig
2012-07-24 14:54           ` 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=4FF2F015.6090109@redhat.com \
    --to=bfoster@redhat.com \
    --cc=david@fromorbit.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.