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

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

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