xfs
[Top] [All Lists]

Re: [PATCH] Revert "xfs: clear PF_NOFREEZE for xfsaild kthread"

To: Michal Hocko <mhocko@xxxxxxx>
Subject: Re: [PATCH] Revert "xfs: clear PF_NOFREEZE for xfsaild kthread"
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 22 Jan 2016 09:24:18 +1100
Cc: xfs@xxxxxxxxxxx, jkosina@xxxxxxx, Hendrik Woltersdorf <hendrikw@xxxxxxxx>, "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160121102129.GD29520@xxxxxxxxxxxxxx>
References: <1452661968-11482-1-git-send-email-david@xxxxxxxxxxxxx> <20160120084750.GA14187@xxxxxxxxxxxxxx> <20160120211944.GE20456@dastard> <20160121102129.GD29520@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jan 21, 2016 at 11:21:30AM +0100, Michal Hocko wrote:
> [CCing Rafael - the thread starts here
> http://lkml.kernel.org/r/1452661968-11482-1-git-send-email-david%40fromorbit.com]
> 
> On Thu 21-01-16 08:19:44, Dave Chinner wrote:
> > On Wed, Jan 20, 2016 at 09:47:50AM +0100, Michal Hocko wrote:
> > > On Wed 13-01-16 16:12:48, Dave Chinner wrote:
> > > > This reverts commit 24ba16bb3d499c49974669cd8429c3e4138ab102 as it
> > > > prevents machines from suspending. This regression occurs when the
> > > > xfsaild is idle on entry to suspend, and so there s no activity to
> > > > wake it from it's idle sleep and hence see that it is supposed to
> > > > freeze. Hence the freezer times out waiting for it and suspend is
> > > > cancelled.
> > > > 
> > > > There is no obvious fix for this short of freezing the filesystem
> > > > properly, so revert this change for now.
> > > 
> > > We had a similar report opensuse bugzilla just recently. I believe the
> > > proper fix should be the following:
> > ....
> > > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> > > index aa67339b9537..d6c9c3e9e02b 100644
> > > --- a/fs/xfs/xfs_trans_ail.c
> > > +++ b/fs/xfs/xfs_trans_ail.c
> > > @@ -520,14 +520,14 @@ xfsaild(
> > >           if (!xfs_ail_min(ailp) &&
> > >               ailp->xa_target == ailp->xa_target_prev) {
> > >                   spin_unlock(&ailp->xa_lock);
> > > -                 schedule();
> > > +                 freezable_schedule();
> > 
> > Oh, wonderful. A completely separate set of undocumented schedule
> > functions defined inside the freezer header files that are needed
> > just for freezable kthreads.
> 
> Yeah, I am not really happy about them either.
> 
> > Perhaps there should be some documentation somewhere on how to write
> > suspend safe kthreads?
> 
> There is something in Documentation/power/freezing-of-tasks.txt but
> freezable_schedule* is not mentioned there. Prhaps something like the
> following would make it more clear?

When I did "git grep freezable_schedule" nothing showed up in the
Documentation directory. I'd suggest that the entire API needs
reference in Documentation/scheduler (i.e. all the
freezable_schedule_timeout variants, the wait_event stuff, etc) as
well as in the freezer documentation so that git grep finds them in
the appropriate places....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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