All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [RFC PATCH v3 2/2] xfs: fix xfsaild hang due to lost wake ups
Date: Thu, 24 May 2012 10:01:48 +1000	[thread overview]
Message-ID: <20120524000148.GO25351@dastard> (raw)
In-Reply-To: <4FBCE081.7050003@redhat.com>

[ Brian, can you line wrap your text at 72 columns? ]

On Wed, May 23, 2012 at 09:05:05AM -0400, Brian Foster wrote:
> On 05/22/2012 08:58 PM, Dave Chinner wrote:
> snip
> 
> > 
> > Hi Brian - here's kind of what I was thinking when we were talking
> > on IRC. basically we move all the idling logic into xfsaild() to
> > keep it out of xfsaild_push(), and make sure we only idle on an
> > empty AIL when we haven't raced with a target update.
> > 
> > So, I was thinking that we add a previous target variable to the
> > xfs_ail structure. Then xfsaild would become something like:
> > 
> > 
> > 	while (!kthread_should_stop()) {
> > 
> > 		spin_lock(&ailp->xa_lock);
> > 		__set_current_state(TASK_INTERRUPTIBLE);
> > 
> > 		/* barrier matches the xa_target update in xfs_ail_push() */
> > 		smp_rmb();
> > 		if (!xfs_ail_min(ailp) && ailp->xa_target == ailp->xa_prev_target) {
> 
> Ok... IIUC, two things can happen here: 1.) we either detect an
> xa_target update and continue on or 2.) if an _ail_push() occurs
> any time between now and when we schedule out, it will issue the
> wakeup successfully because we've already set the task state above
> (thus avoiding the race).

Exactly.

> > FWIW, you might be able to do this without the idle wait queue
> > and just use wake_up_process() - 
> > 
> 
> Ok... I'll look into using a wait queue once I have the basics
> working as is and put the whole thing through my reproducer.

Ah, I forgot to remove that line from the email before I sent it. I
originally thought an idle wake queue would be necessary, but then
realised it wasn't and removed it from the code I wrote
above. So, no, and idle wait queue is not necessary....

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-05-24  0:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-22 16:38 [RFC PATCH v3 0/2] xfs: fix xfsaild races and re-enable idle mode Brian Foster
2012-05-22 16:38 ` [RFC PATCH v3 1/2] xfs: re-enable xfsaild idle mode when the ail is empty Brian Foster
2012-05-22 16:38 ` [RFC PATCH v3 2/2] xfs: fix xfsaild hang due to lost wake ups Brian Foster
2012-05-23  0:58   ` Dave Chinner
2012-05-23 13:05     ` Brian Foster
2012-05-24  0:01       ` Dave Chinner [this message]
2012-05-23 17:48     ` Brian Foster
2012-05-23 18:19       ` Mark Tinguely
2012-05-23 23:41         ` Brian Foster
2012-05-23 23:53         ` Dave Chinner
2012-05-24 14:38           ` Mark Tinguely
2012-05-24  0:06       ` Dave Chinner
2012-05-24 13:07         ` Brian Foster

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=20120524000148.GO25351@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.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.