xfs
[Top] [All Lists]

Re: [PATCH 3/4] xfs: revert to using a kthread for AIL pushing

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/4] xfs: revert to using a kthread for AIL pushing
From: Markus Trippelsdorf <markus@xxxxxxxxxxxxxxx>
Date: Mon, 10 Oct 2011 07:55:46 +0200
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Tejun Heo <tj@xxxxxxxxxx>, Stefan Priebe <s.priebe@xxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=simple; d=mail.ud10.udmedia.de; h= date:from:to:cc:subject:message-id:references:mime-version: content-type:in-reply-to; q=dns/txt; s=beta; bh=fIbrHS04a8bWGJE7 KkxSmKR06LJwuIr/E4kFw56+sN4=; b=UXNl6cB8FwpuTi1I4+VCNj7/SlMvRh3y 6JJaYVRo+pDO1BtlE3OOM3eLlW8Oyx9PFp6/TYvCv2SU5O7aW2PY8r+CvO07RP/R eRJKHvc//y8suJIsBvPgPlmnzA/OTDkULHiXRd1g93uQ+RAhz4h/DHNXL3XuBLax u/DAEQycPCA=
In-reply-to: <20111010014509.GT3159@dastard>
References: <20111006183257.036884724@xxxxxxxxxxxxxxxxxxxxxx> <20111006183549.770414484@xxxxxxxxxxxxxxxxxxxxxx> <20111010014509.GT3159@dastard>
On 2011.10.10 at 12:45 +1100, Dave Chinner wrote:
> On Thu, Oct 06, 2011 at 02:33:00PM -0400, Christoph Hellwig wrote:
> > Currently we have a few issues with the way the workqueue code is used to
> > implement AIL pushing:
> > 
> >  - it accidentally uses the same workqueue as the syncer action, and thus
> >    can be prevented from running if there are enough sync actions active
> >    in the system.
> >  - it doesn't use the HIGHPRI flag to queue at the head of the queue of
> >    work items
> > 
> > At this point I'm not confident enough in getting all the workqueue flags 
> > and
> > tweaks right to provide a perfectly reliable execution context for AIL
> > pushing, which is the most important piece in XFS to make forward progress
> > when the log fills.
> > 
> > Revert back to use a kthread per filesystem which fixes all the above issues
> > at the cost of having a task struct and stack around for each mounted
> > filesystem.  In addition this also gives us much better ways to diagnose
> > any issues involving hung AIL pushing and removes a small amount of code.
> > 
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > Reported-by: Stefan Priebe <s.priebe@xxxxxxxxxxxx>
> > Tested-by: Stefan Priebe <s.priebe@xxxxxxxxxxxx>
> 
> I'd much prefer to fix the problems with the workqueue usage than
> revert back to using a thread, but seeing as I cannot reproduce the
> hangs I can't really track down whatever problem there is. So,
> a bit reluctantly:

Wouldn't it be possible to verify that the problem also goes away with
this simple one liner?

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 2366c54..daf30c9 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1654,7 +1654,7 @@ xfs_init_workqueues(void)
        if (!xfs_syncd_wq)
                goto out;
 
-       xfs_ail_wq = alloc_workqueue("xfsail", WQ_CPU_INTENSIVE, 8);
+       xfs_ail_wq = alloc_workqueue("xfsail", WQ_HIGHPRI | WQ_CPU_INTENSIVE, 
8);
        if (!xfs_ail_wq)
                goto out_destroy_syncd;
 
>From Documentation/workqueue.txt:

  WQ_HIGHPRI | WQ_CPU_INTENSIVE

        This combination makes the wq avoid interaction with
        concurrency management completely and behave as a simple
        per-CPU execution context provider.  Work items queued on a
        highpri CPU-intensive wq start execution as soon as resources
        are available and don't affect execution of other work items.

So this should be identical to reverting back to the kthread. No?
CCing Tejun, maybe he can comment on this?

-- 
Markus

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