xfs
[Top] [All Lists]

Re: [RFC PATCH v3 2/2] xfs: fix xfsaild hang due to lost wake ups

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH v3 2/2] xfs: fix xfsaild hang due to lost wake ups
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 24 May 2012 10:01:48 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <4FBCE081.7050003@xxxxxxxxxx>
References: <1337704714-50235-1-git-send-email-bfoster@xxxxxxxxxx> <1337704714-50235-3-git-send-email-bfoster@xxxxxxxxxx> <20120523005830.GL25351@dastard> <4FBCE081.7050003@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
[ 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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>