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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 23, 2012 at 01:48:54PM -0400, Brian Foster wrote:
> On 05/22/2012 08:58 PM, Dave Chinner wrote: snip
> > 
> > Finally, rather than calling wake_up_process() in the
> > xfs_ail_push*() functions, call wake_up(&ailp->xa_idle); There
> > can only be one thread sleeping on that (the xfsaild) so there
> > is no need to use the wake_up_all() variant...
> > 
> > FWIW, you might be able to do this without the idle wait queue
> > and just use wake_up_process() - 
> > 
> 
> Hi Dave,
> 
> I have a working version of your suggested algorithm. It looks
> mostly the same with the exception of a spin_unlock fix. I also
> have the below version that uses a wait_queue and that I plan to
> test overnight tonight:

See my previous mail about using an idle queue.

> 	while (!kthread_should_stop()) {
> 		if (tout && tout <= 20)
> 			state = TASK_KILLABLE;
> 		else
> 			state = TASK_INTERRUPTIBLE;
> 
> 		prepare_to_wait(&ailp->xa_idle, &wait, state);
> 
> 		spin_lock(&ailp->xa_lock);
> 		/* barrier matches the xa_target update in xfs_ail_push() */
> 		smp_rmb();
> 		if (!xfs_ail_min(ailp) && (ailp->xa_target == ailp->xa_target_prev)) {
> 			/* the ail is empty and no change to the push target - idle */
> 			spin_unlock(&ailp->xa_lock);
> 			schedule();
> 		} else if (tout) {
> 			spin_unlock(&ailp->xa_lock);
> 			/* more work to do soon */
> 			schedule_timeout(msecs_to_jiffies(tout));
> 		} else {
> 			spin_unlock(&ailp->xa_lock);
> 		}

Three separate unlocks? that's a recipe for future disasters. how
about:

		if (!xfs_ail_min(ailp) && (ailp->xa_target == ailp->xa_target_prev)) {
			/* the ail is empty and no change to the push target - idle */
			spin_unlock(&ailp->xa_lock);
			schedule();
			tout = 0;
			continue;
		}
		spin_unlock(&ailp->xa_lock);

		if (tout) {
			/* more work to do soon */
			schedule_timeout(msecs_to_jiffies(tout));
		}

So that we recheck the idle condition on wakeup from idle before
doing anything. (i.e. handle spurious idle wakeups effectively). By
setting the tout to zero, we then fall through immediately to
pushing the AIL if it was a real wakeup that moved the target....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux