[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? diff --git a/Documentation/power/freezing-of-tasks.txt b/Documentation/power/freezing-of-tasks.txt index 85894d83b352..4df53f0c05a6 100644 --- a/Documentation/power/freezing-of-tasks.txt +++ b/Documentation/power/freezing-of-tasks.txt @@ -56,8 +56,11 @@ calling try_to_freeze(). The main loop of a freezable kernel thread may look If a freezable kernel thread fails to call try_to_freeze() after the freezer has initiated a freezing operation, the freezing of tasks will fail and the entire hibernation operation will be cancelled. For this reason, freezable kernel -threads must call try_to_freeze() somewhere or use one of the -wait_event_freezable() and wait_event_freezable_timeout() macros. +threads must call try_to_freeze() somewhere, use wait_event_freezable*() +and wait_event_freezable_timeout() macros or use freezable_schedule* to hide +the kernel thread from the suspend if it is guaranteed that a later wake up +cannot interact with the suspend process (e.g. there is a try_to_freeze call +or no IO is submitted or any of the suspended devices is not accessed etc..) After the system memory state has been restored from a hibernation image and devices have been reinitialized, the function thaw_processes() is called in > This is too late for the current merge window, but I'll queue it up > for the next one so we can get some testing on it first. Sure, no problem and thanks! -- Michal Hocko SUSE Labs _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
