xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [RFC PATCH v3 2/2] xfs: fix xfsaild hang due to lost wake ups
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Wed, 23 May 2012 13:48:54 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120523005830.GL25351@dastard>
References: <1337704714-50235-1-git-send-email-bfoster@xxxxxxxxxx> <1337704714-50235-3-git-send-email-bfoster@xxxxxxxxxx> <20120523005830.GL25351@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120424 Thunderbird/12.0
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:

        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);
                }

                finish_wait(&ailp->xa_idle, &wait);

                try_to_freeze();

                tout = xfsaild_push(ailp);
        }

... and obviously the xfs_ail_push() side changes to:

        wake_up(&ailp->xa_idle);

Does this wait_queue version look sane to you? Thanks again..

Brian

> Cheers,
> 
> Dave.

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