Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Yu Chen]

On Mon, Nov 13, 2017 at 09:14:14PM +0100, Luis R. Rodriguez wrote:
> On Mon, Nov 13, 2017 at 06:31:39PM +0800, Yu Chen wrote:
> > The xfs-buf/dm-1 should be freezed according to
> > commit 8018ec083c72 ("xfs: mark all internal workqueues
> > as freezable"), thus a easier way might be have to revert
> > commit 18f1df4e00ce ("xfs: Make xfsaild freezeable
> > again") for now, after this reverting the xfsaild/dm-1
> > becomes non-freezable again, thus pm does not see this
> > thread - unless we find a graceful way to treat xfsaild/dm-1
> > as 'frozen' if it is waiting for an already 'frozen' task,
> > or if the filesystem freeze is added in.
> > 
> > Any comments would be much appreciated.
> 
> Reverting 18f1df4e00ce ("xfs: Make xfsaild freezeable again")
> would break the proper form of the kthread for it to be freezable.
> This "form" is not defined formally, and sadly its just a form
> learned throughout years over different kthreads in the kernel.
> 
> I'm also not convinced all our hibernation / suspend woes would be fixed by
> reverting this commit, its why I worked instead on formalizing a proper freeze
> / thaw, which a lot of filesystems already implement prior to system
> hibernation / suspend / resume [0].
> 
> I'll be respinning this series without the last patch, provided I'm able to
> ensure I don't need the ext[234] hack I did in that thread. Can you test the
> first 3 patches *only* on that series and seeing if that helps on your XFS
> front as well?
> 
> [0] https://lkml.kernel.org/r/20171003185313.1017-1-mcgrof@kernel.org
> 
>   Luis
Thanks for the comment Luis,
Yes, I agree the freezing of filesystem is a proper/thorough fix for such
kind issues, but as Dan said, it might be a little risky for us to
to deploy it on our products currently, unless it is in the
mainline/stable branch. Although the XFS issue might not be 100% reproducible,
we can help test the patch set while seeking for a lightweight 'fix'.
Thanks,
	Yu

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Luis R. Rodriguez]

On Mon, Nov 13, 2017 at 12:37:00PM -0800, Dan Williams wrote:
> On Mon, Nov 13, 2017 at 12:14 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Mon, Nov 13, 2017 at 06:31:39PM +0800, Yu Chen wrote:
> >> The xfs-buf/dm-1 should be freezed according to
> >> commit 8018ec083c72 ("xfs: mark all internal workqueues
> >> as freezable"), thus a easier way might be have to revert
> >> commit 18f1df4e00ce ("xfs: Make xfsaild freezeable
> >> again") for now, after this reverting the xfsaild/dm-1
> >> becomes non-freezable again, thus pm does not see this
> >> thread - unless we find a graceful way to treat xfsaild/dm-1
> >> as 'frozen' if it is waiting for an already 'frozen' task,
> >> or if the filesystem freeze is added in.
> >>
> >> Any comments would be much appreciated.
> >
> > Reverting 18f1df4e00ce ("xfs: Make xfsaild freezeable again")
> > would break the proper form of the kthread for it to be freezable.
> > This "form" is not defined formally, and sadly its just a form
> > learned throughout years over different kthreads in the kernel.
> 
> If the behavior breaks then the "form" is broken.

People have been arguing this for a long time, as such a long term objective is
to do away with kthread freezing all together. That's easier said than done.
It will require some more work though, so a tiny initial first goal will be to
do fs freezing prior to suspend/hibernate. For discussions on the challenges on
removing kthread frerezing, you can refer to the discussion of the last 2
patches in my series.

> > I'm also not convinced all our hibernation / suspend woes would be fixed by
> > reverting this commit, its why I worked instead on formalizing a proper freeze
> > / thaw, which a lot of filesystems already implement prior to system
> > hibernation / suspend / resume [0].
> >
> > I'll be respinning this series without the last patch, provided I'm able to
> > ensure I don't need the ext[234] hack I did in that thread. Can you test the
> > first 3 patches *only* on that series and seeing if that helps on your XFS
> > front as well?
> 
> Those do not seem suitable for a  -stable backport.

Agreed. But fortunately filesystem freeze support has been in place for XFS
since v2.6.29 via commit c4be0c1dc4cdc ("filesystem freeze: add error handling
of write_super_lockfs/unlockfs") and likewise for a few other filesystems.
Prior to this commit freeze_fs() was write_super_lockfs() and returned
void, likewise unfreeze_fs() was unlockfs() and also returned void, after
the commit both could return an error. So one option for stable kernels
is to see if you can devise a userspace hook pre suspend or hiberanate
to issue the freeze fs yourself, and back:

/usr/sbin/xfs_freeze /mount-point
/usr/sbin/xfs_freeze -u /mount-point

In systemd-isneyverse, as per systemd-suspend.service(8), this *could* be
something like stashing the following script the systemd-suspend.service(8)
special path, for instance my systemd-suspend.service(8) on openSUSE
is /usr/lib/systemd/system-sleep/ however on Debian this is
   /lib/systemd/system-sleep/

So say you stash a fs.sleep script[0] there, first verify:

systemctl status systemd-suspend.service

You can then debug this while suspending with:

journalctl -f -u systemd-suspend

This is not enough though, one would also have to suspend processes
mucking with XFS filesystems, and unfortunately there isn't much of
graceful way to do this from userspace I think that I could come up
with except using SIGSTOP, and using lsofs on the mount point...

This seems to work on both my OpenSUSE and Debian systems, except the user
experience may not be as ideal, and this is precisely why having the kernel do
this work is much better long term.

#!/bin/bash

set -e

XFS_FREEZE="/usr/sbin/xfs_freeze"
PROC_MOUNTS="/proc/mounts"
SUSPEND_SIGNAL="SIGSTOP"

error_quit()
{
	echo "$1" >&2
	exit 1
}

check-system()
{
	[ -r "${PROC_MOUNTS}" ] || error_quit "ERROR: cannot find or read ${PROC_MOUNTS}"
	[ -x "${XFS_FREEZE}" ] || error_quit "ERROR: cannot find or execute ${XFS_FREEZE}"
}

run-fs-freeze()
{
	local i FSTYPE MNT ROOTDEV ARGS

	while read ROOTDEV MNT FSTYPE ARGS; do
	    [ "$ROOTDEV" = "rootfs" ] && continue
	    [ "$MNT" = "/" ] && continue

	    case $FSTYPE in
	    xfs)
		echo "  Trying to freeze userspace processes on '$FSTYPE' mounted on ${MNT}... "
		for i in $(lsof +D $MNT -t 2>/dev/null); do kill -$SUSPEND_SIGNAL $i; done
		echo -n "  Trying to freeze fstype '$FSTYPE' mounted on ${MNT}... "
		$XFS_FREEZE -f $MNT
		echo "OK!"
		;;
	    *)
		;;
	    esac

	done < $PROC_MOUNTS
}

run-fs-unfreeze()
{
	local i FSTYPE MNT ROOTDEV ARGS

	while read ROOTDEV MNT FSTYPE ARGS; do
	    [ "$ROOTDEV" = "rootfs" ] && continue
	    [ "$MNT" = "/" ] && continue

	    case $FSTYPE in
	    xfs)
		echo "  Trying to unfreeze userspace processes on '$FSTYPE' mounted on ${MNT}... "
		for i in $(lsof +D $MNT -t 2>/dev/null); do kill -SIGCONT $i; done
		echo -n "  Trying to unfreeze fstype '$FSTYPE' mounted on ${MNT}... "
		$XFS_FREEZE -u $MNT
		echo "OK!"
		;;
	    *)
		;;
	    esac

	done < $PROC_MOUNTS
}

check-system

if [ "$2" = suspend ]; then
	echo "INFO: running $0 for $2"
else
	echo "INFO: running $0 for $2"
fi

if [ "$1" = pre ] ; then
	run-fs-freeze
fi
if [ "$1" = post ] ; then
	run-fs-unfreeze
fi

> We can always
> follow on with these patches once -stable and mainline are back to
> their baseline behavior.

Reverting 18f1df4e00ce ("xfs: Make xfsaild freezeable again") cannot
be defined as "going back to baseline behavior" given it also implies
we'd be resurfacing the issue Hendrik reported last year in February,
that of suspend failures due to xfsaild blocking the freezer to
settle down:

    Jan 17 19:59:56 linux-6380 kernel: PM: Syncing filesystems ... done.
    Jan 17 19:59:56 linux-6380 kernel: PM: Preparing system for sleep (mem)
    Jan 17 19:59:56 linux-6380 kernel: Freezing user space processes ... (elapsed 0.001 seconds) done.
    Jan 17 19:59:56 linux-6380 kernel: Freezing remaining freezable tasks ...
    Jan 17 19:59:56 linux-6380 kernel: Freezing of tasks failed after 20.002 seconds (1 tasks refusing to freeze, wq_busy=0):
    Jan 17 19:59:56 linux-6380 kernel: xfsaild/dm-5    S 00000000     0  1293      2 0x00000080
    Jan 17 19:59:56 linux-6380 kernel:  f0ef5f00 00000046 00000200 00000000 ffff9022 c02d3800 00000000 00000032
    Jan 17 19:59:56 linux-6380 kernel:  ee0b2400 00000032 f71e0d00 f36fabc0 f0ef2d00 f0ef6000 f0ef2d00 f12f90c0
    Jan 17 19:59:56 linux-6380 kernel:  f0ef5f0c c0844e44 00000000 f0ef5f6c f811e0be 00000000 00000000 f0ef2d00
    Jan 17 19:59:56 linux-6380 kernel: Call Trace:
    Jan 17 19:59:56 linux-6380 kernel:  [<c0844e44>] schedule+0x34/0x90
    Jan 17 19:59:56 linux-6380 kernel:  [<f811e0be>] xfsaild+0x5de/0x600 [xfs]
    Jan 17 19:59:56 linux-6380 kernel:  [<c0286cbb>] kthread+0x9b/0xb0
    Jan 17 19:59:56 linux-6380 kernel:  [<c0848a79>] ret_from_kernel_thread+0x21/0x38

As noted on the commit log of Michal's patch, the issue had been there for quite
some time, its just commit 24ba16bb3d4 ("xfs: clear PF_NOFREEZE for xfsaild
kthread") made the issue visible.

This is another way to say suspend has been busted on XFS for a very long time,
but I would not blame XFS -- this is a kernel issue to get proper filesystem
suspend working right, and the way we currently deal with kthreads is just
a sloppy goo mess which has created this situation.

  Luis

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Dave Chinner]

On Tue, Nov 14, 2017 at 09:19:15PM +0100, Luis R. Rodriguez wrote:
> This is another way to say suspend has been busted on XFS for a very long time,
> but I would not blame XFS -- this is a kernel issue to get proper filesystem
> suspend working right, and the way we currently deal with kthreads is just
> a sloppy goo mess which has created this situation.

Yes, and I've been telling people that suspend on journalling
filesystems has been broken for a long time (i.e since I first
realised the scope of the problem back in 2005). However, only XFS
triggers those conditions regularly because it is the most
asynchronous of the "freezable" journalling filesytems and has the
most reliance on co-ordination of kernel threads and workqueues to
function correctly.

IOWs, suspend of filesystems has been broken forever, and we've been
slapping bandaids on it in XFS forever. Now we've got to a
catch-22 situation that bandaids can't fix. We need structural
fixes, like I said we needed to do more than 10 years ago.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Rafael J. Wysocki]

On Tuesday, November 14, 2017 10:25:38 PM CET Dave Chinner wrote:
> On Tue, Nov 14, 2017 at 09:19:15PM +0100, Luis R. Rodriguez wrote:
> > This is another way to say suspend has been busted on XFS for a very long time,
> > but I would not blame XFS -- this is a kernel issue to get proper filesystem
> > suspend working right, and the way we currently deal with kthreads is just
> > a sloppy goo mess which has created this situation.
> 
> Yes, and I've been telling people that suspend on journalling
> filesystems has been broken for a long time (i.e since I first
> realised the scope of the problem back in 2005). However, only XFS
> triggers those conditions regularly because it is the most
> asynchronous of the "freezable" journalling filesytems and has the
> most reliance on co-ordination of kernel threads and workqueues to
> function correctly.
> 
> IOWs, suspend of filesystems has been broken forever, and we've been
> slapping bandaids on it in XFS forever. Now we've got to a
> catch-22 situation that bandaids can't fix. We need structural
> fixes, like I said we needed to do more than 10 years ago.

Right.

Yet, apparently, nobody has had the time to make those changes for all
that time.

So what do you think should be done, specifically?

Thanks,
Rafael

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Dave Chinner]

On Tue, Nov 14, 2017 at 11:01:57PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 14, 2017 10:25:38 PM CET Dave Chinner wrote:
> > On Tue, Nov 14, 2017 at 09:19:15PM +0100, Luis R. Rodriguez wrote:
> > > This is another way to say suspend has been busted on XFS for a very long time,
> > > but I would not blame XFS -- this is a kernel issue to get proper filesystem
> > > suspend working right, and the way we currently deal with kthreads is just
> > > a sloppy goo mess which has created this situation.
> > 
> > Yes, and I've been telling people that suspend on journalling
> > filesystems has been broken for a long time (i.e since I first
> > realised the scope of the problem back in 2005). However, only XFS
> > triggers those conditions regularly because it is the most
> > asynchronous of the "freezable" journalling filesytems and has the
> > most reliance on co-ordination of kernel threads and workqueues to
> > function correctly.
> > 
> > IOWs, suspend of filesystems has been broken forever, and we've been
> > slapping bandaids on it in XFS forever. Now we've got to a
> > catch-22 situation that bandaids can't fix. We need structural
> > fixes, like I said we needed to do more than 10 years ago.
> 
> Right.
> 
> Yet, apparently, nobody has had the time to make those changes for all
> that time.

No, that's not the case. The problem was that the suspend developers
were in complete denial about the short-comings of using "sync" to
quiesce filesystems.  It wasn't until Plumbers a year ago that it
was reluctantly admitted that maybe there was a problem and that
filesystem freeze should be used instead.

Until the admission that "sync" was inadequate was made, there was
no point in even proposing a solution; the response up until that
point in time was a "we don't see a problem here" denial. We had to
wait for you guys to come to that conclusion yourselves because you
simply wouldn't listen to what the filesystem developers were
repeatedly telling you....

> So what do you think should be done, specifically?

Luis posted an initial version of those "use fs freeze" changes
recently, so Work with Luis to get those changes into the kernel.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Rafael J. Wysocki]

On Wednesday, November 15, 2017 12:07:30 AM CET Dave Chinner wrote:
> On Tue, Nov 14, 2017 at 11:01:57PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, November 14, 2017 10:25:38 PM CET Dave Chinner wrote:
> > > On Tue, Nov 14, 2017 at 09:19:15PM +0100, Luis R. Rodriguez wrote:
> > > > This is another way to say suspend has been busted on XFS for a very long time,
> > > > but I would not blame XFS -- this is a kernel issue to get proper filesystem
> > > > suspend working right, and the way we currently deal with kthreads is just
> > > > a sloppy goo mess which has created this situation.
> > > 
> > > Yes, and I've been telling people that suspend on journalling
> > > filesystems has been broken for a long time (i.e since I first
> > > realised the scope of the problem back in 2005). However, only XFS
> > > triggers those conditions regularly because it is the most
> > > asynchronous of the "freezable" journalling filesytems and has the
> > > most reliance on co-ordination of kernel threads and workqueues to
> > > function correctly.
> > > 
> > > IOWs, suspend of filesystems has been broken forever, and we've been
> > > slapping bandaids on it in XFS forever. Now we've got to a
> > > catch-22 situation that bandaids can't fix. We need structural
> > > fixes, like I said we needed to do more than 10 years ago.
> > 
> > Right.
> > 
> > Yet, apparently, nobody has had the time to make those changes for all
> > that time.
> 
> No, that's not the case. The problem was that the suspend developers
> were in complete denial about the short-comings of using "sync" to
> quiesce filesystems.  It wasn't until Plumbers a year ago that it
> was reluctantly admitted that maybe there was a problem and that
> filesystem freeze should be used instead.

Excuse me, but that's not correct.

Fortunately, the old stuff is still there:

https://lists.celinuxforum.org/pipermail/linux-pm/2011-August/032513.html

and you are on the thread.  That was in 2011 and it failed, unfortunately.

I needed help at that time and didn't have the time to figure it all out
myself, but you know what, nobody from the fs side helped me then.

Next time Jiri tried to revive this, in 2015 AFAICS:

https://marc.info/?l=linux-kernel&m=144621283608619&w=2

and again, it didn't get too far.

> Until the admission that "sync" was inadequate was made, there was
> no point in even proposing a solution; the response up until that
> point in time was a "we don't see a problem here" denial. We had to
> wait for you guys to come to that conclusion yourselves because you
> simply wouldn't listen to what the filesystem developers were
> repeatedly telling you....

Can we stop here and be constructive going forward, please?

> > So what do you think should be done, specifically?
> 
> Luis posted an initial version of those "use fs freeze" changes
> recently, so Work with Luis to get those changes into the kernel.

This isn't the first version as per the above and sorry, but without
help from fs people this is going to end the same way as before and
I sort of doubt that "working with Luis" alone is going to be sufficient.

We need the fs people to actually help us, pretty much like back in 2011.

Thanks,
Rafael

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Luis R. Rodriguez]

On Wed, Nov 15, 2017 at 12:40:43AM +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 15, 2017 12:07:30 AM CET Dave Chinner wrote:
> Can we stop here and be constructive going forward, please?

Then let's stop this nonsense and get to work.

> > > So what do you think should be done, specifically?
> > 
> > Luis posted an initial version of those "use fs freeze" changes
> > recently, so Work with Luis to get those changes into the kernel.
> 
> This isn't the first version as per the above and sorry, but without
> help from fs people this is going to end the same way as before and
> I sort of doubt that "working with Luis" alone is going to be sufficient.
> 
> We need the fs people to actually help us, pretty much like back in 2011.

Rafael, AFAICT the only outstanding issue with the patches I posted were
the last two patches. I haven't had time to re-test my patches without the
last patch, but if Chinner was right, and I suspect he was, then the last patch
is what implicated the need for the ext patch, which we don't want. So dropping
the last two patches I think would be a good starting point.

I hope to have time to re-test this week, if you feel compelled to help,
you can also help test the patches except for the last two. Then if you
want to help further, helping iron out the kinks on the last patch would
be much appreciated.

  Luis

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Rafael J. Wysocki]

On Wed, Nov 15, 2017 at 7:01 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Wed, Nov 15, 2017 at 12:40:43AM +0100, Rafael J. Wysocki wrote:
>> On Wednesday, November 15, 2017 12:07:30 AM CET Dave Chinner wrote:
>> Can we stop here and be constructive going forward, please?
>
> Then let's stop this nonsense and get to work.
>
>> > > So what do you think should be done, specifically?
>> >
>> > Luis posted an initial version of those "use fs freeze" changes
>> > recently, so Work with Luis to get those changes into the kernel.
>>
>> This isn't the first version as per the above and sorry, but without
>> help from fs people this is going to end the same way as before and
>> I sort of doubt that "working with Luis" alone is going to be sufficient.
>>
>> We need the fs people to actually help us, pretty much like back in 2011.
>
> Rafael, AFAICT the only outstanding issue with the patches I posted were
> the last two patches. I haven't had time to re-test my patches without the
> last patch, but if Chinner was right, and I suspect he was, then the last patch
> is what implicated the need for the ext patch, which we don't want. So dropping
> the last two patches I think would be a good starting point.
>
> I hope to have time to re-test this week, if you feel compelled to help,
> you can also help test the patches except for the last two. Then if you
> want to help further, helping iron out the kinks on the last patch would
> be much appreciated.

OK, please resend the series when you're ready.

Thanks,
Rafael

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Yu Chen]

Hi Dave,
On Tue, Nov 14, 2017 at 09:52:16AM +1100, Dave Chinner wrote:
> On Mon, Nov 13, 2017 at 06:31:39PM +0800, Yu Chen wrote:
> > Hi all,
> > Currently we are running hibernation stress test on a server
> > and unfortunately after 48 rounds of cycling, it fails at a
> > early stage that, the xfs task refuses to be frozen by the system:
> > 
> > [ 1934.221653] PM: Syncing filesystems ...
> > [ 1934.661517] PM: done.
> > [ 1934.664067] Freezing user space processes ... (elapsed 0.003 seconds) done.
> > [ 1934.675251] OOM killer disabled.
> > [ 1934.724317] PM: Preallocating image memory... done (allocated 6906555 pages)
> > [ 1954.666378] PM: Allocated 27626220 kbytes in 19.93 seconds (1386.16 MB/s)
> > [ 1954.673939] Freezing remaining freezable tasks ...
> > [ 1974.681089] Freezing of tasks failed after 20.001 seconds (1 tasks refusing to freeze, wq_busy=0):
> > [ 1974.691169] xfsaild/dm-1    D    0  1362      2 0x00000080
> > [ 1974.697283] Call Trace:
> > [ 1974.700014]  __schedule+0x3be/0x830
> > [ 1974.703898]  schedule+0x36/0x80
> > [ 1974.707440]  _xfs_log_force+0x143/0x280 [xfs]
> > [ 1974.712295]  ? schedule_timeout+0x16b/0x350
> > [ 1974.716953]  ? wake_up_q+0x80/0x80
> > [ 1974.720752]  ? xfsaild+0x16f/0x770 [xfs]
> > [ 1974.725134]  xfs_log_force+0x2c/0x80 [xfs]
> > [ 1974.729707]  xfsaild+0x16f/0x770 [xfs]
> > [ 1974.733885]  kthread+0x109/0x140
> > [ 1974.737480]  ? kthread+0x109/0x140
> > [ 1974.741271]  ? xfs_trans_ail_cursor_first+0x90/0x90 [xfs]
> > [ 1974.747284]  ? kthread_park+0x60/0x60
> > [ 1974.751354]  ret_from_fork+0x25/0x30
> > [ 1974.755366] Restarting kernel threads ... done.
> > [ 1978.259907] OOM killer enabled.
> > [ 1978.263405] Restarting tasks ... done.
> > 
> > The reason for this failure might be that,
> > while the kernel thread xfsaild/dm-1 is waiting for
> > xfs-buf/dm-1 to wake it up, however the latter
> > has already been frozen, thus xfsaild/dm-1 never
> > has a chance to be woken up and get froze. (Although
> > the xfsaild/dm-1 remains in TASK_UNINTERRUPTIBLE, which
> > is quite similar to 'frozen'.)
> 
> Should be fixed by this commit in the for-next branch:
> 
> 0bd89676c4fe xfs: check kthread_should_stop() after the setting of task state
> 
> That should get merged into 4.15 with the next merge...
>
I did not quite catch why above commit would fix the issue here,
according to
https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=0bd89676c4fed53b003025bc4a5200861ac5d8ef
it tries to address a race condition between umount and xfsaild on
checking the kthread_should_stop() in order not to make
xfsaild falling asleep indefinitely.
But in our case, the xfsaild is waiting for the xfs-buf to wake
it up, and is nothing related to the kthread_should_stop() checking
here, did I miss something?
Thanks,
	Yu

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Dave Chinner]

On Tue, Nov 14, 2017 at 11:39:59AM +0800, Yu Chen wrote:
> Hi Dave,
> On Tue, Nov 14, 2017 at 09:52:16AM +1100, Dave Chinner wrote:
> > On Mon, Nov 13, 2017 at 06:31:39PM +0800, Yu Chen wrote:
> > > Hi all,
> > > Currently we are running hibernation stress test on a server
> > > and unfortunately after 48 rounds of cycling, it fails at a
> > > early stage that, the xfs task refuses to be frozen by the system:
> > > 
> > > [ 1934.221653] PM: Syncing filesystems ...
> > > [ 1934.661517] PM: done.
> > > [ 1934.664067] Freezing user space processes ... (elapsed 0.003 seconds) done.
> > > [ 1934.675251] OOM killer disabled.
> > > [ 1934.724317] PM: Preallocating image memory... done (allocated 6906555 pages)
> > > [ 1954.666378] PM: Allocated 27626220 kbytes in 19.93 seconds (1386.16 MB/s)
> > > [ 1954.673939] Freezing remaining freezable tasks ...
> > > [ 1974.681089] Freezing of tasks failed after 20.001 seconds (1 tasks refusing to freeze, wq_busy=0):
> > > [ 1974.691169] xfsaild/dm-1    D    0  1362      2 0x00000080
> > > [ 1974.697283] Call Trace:
> > > [ 1974.700014]  __schedule+0x3be/0x830
> > > [ 1974.703898]  schedule+0x36/0x80
> > > [ 1974.707440]  _xfs_log_force+0x143/0x280 [xfs]
> > > [ 1974.712295]  ? schedule_timeout+0x16b/0x350
> > > [ 1974.716953]  ? wake_up_q+0x80/0x80
> > > [ 1974.720752]  ? xfsaild+0x16f/0x770 [xfs]
> > > [ 1974.725134]  xfs_log_force+0x2c/0x80 [xfs]
> > > [ 1974.729707]  xfsaild+0x16f/0x770 [xfs]
> > > [ 1974.733885]  kthread+0x109/0x140
> > > [ 1974.737480]  ? kthread+0x109/0x140
> > > [ 1974.741271]  ? xfs_trans_ail_cursor_first+0x90/0x90 [xfs]
> > > [ 1974.747284]  ? kthread_park+0x60/0x60
> > > [ 1974.751354]  ret_from_fork+0x25/0x30
> > > [ 1974.755366] Restarting kernel threads ... done.
> > > [ 1978.259907] OOM killer enabled.
> > > [ 1978.263405] Restarting tasks ... done.
> > > 
> > > The reason for this failure might be that,
> > > while the kernel thread xfsaild/dm-1 is waiting for
> > > xfs-buf/dm-1 to wake it up, however the latter
> > > has already been frozen, thus xfsaild/dm-1 never
> > > has a chance to be woken up and get froze. (Although
> > > the xfsaild/dm-1 remains in TASK_UNINTERRUPTIBLE, which
> > > is quite similar to 'frozen'.)
> > 
> > Should be fixed by this commit in the for-next branch:
> > 
> > 0bd89676c4fe xfs: check kthread_should_stop() after the setting of task state
> > 
> > That should get merged into 4.15 with the next merge...
> >
> I did not quite catch why above commit would fix the issue here,
> according to
> https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=0bd89676c4fed53b003025bc4a5200861ac5d8ef
> it tries to address a race condition between umount and xfsaild on
> checking the kthread_should_stop() in order not to make
> xfsaild falling asleep indefinitely.

Argh, got my threads slightly crossed there.

> But in our case, the xfsaild is waiting for the xfs-buf to wake
> it up, and is nothing related to the kthread_should_stop() checking
> here, did I miss something?

Similar symptoms - the symptom that was fixed by the commit I
mentioned was the xfsaild getting stuck in sleeping forever and so
never seeing the KTHREAD_STOP bit - it was a "set bit vs wakeup"
race caused by the fact that we didn't reset the state of the
task correctly after wakeup.

You said:

>> (Although the xfsaild/dm-1 remains in TASK_UNINTERRUPTIBLE, which
>> is quite similar to 'frozen'.)

So from a quick look, it seemed like a similar race condition. I
missed the *un* part of the task state, though.
TASK_UNINTERRUPTIBLE implies waiting for IO completion, which is
what _xfs_log_force() is doing.

SO, follow the other branch of the discussion: hibernation needs to
freeze filesystems so they can quiesce gracefully before the kernel
starts shutting down the infrastructure the filesystem relies on...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Yu Chen]

On Tue, Nov 14, 2017 at 03:02:32PM +1100, Dave Chinner wrote:
> On Tue, Nov 14, 2017 at 11:39:59AM +0800, Yu Chen wrote:
> > Hi Dave,
> > On Tue, Nov 14, 2017 at 09:52:16AM +1100, Dave Chinner wrote:
> > > On Mon, Nov 13, 2017 at 06:31:39PM +0800, Yu Chen wrote:
> > > > Hi all,
> > > > Currently we are running hibernation stress test on a server
> > > > and unfortunately after 48 rounds of cycling, it fails at a
> > > > early stage that, the xfs task refuses to be frozen by the system:
> > > > 
> > > > [ 1934.221653] PM: Syncing filesystems ...
> > > > [ 1934.661517] PM: done.
> > > > [ 1934.664067] Freezing user space processes ... (elapsed 0.003 seconds) done.
> > > > [ 1934.675251] OOM killer disabled.
> > > > [ 1934.724317] PM: Preallocating image memory... done (allocated 6906555 pages)
> > > > [ 1954.666378] PM: Allocated 27626220 kbytes in 19.93 seconds (1386.16 MB/s)
> > > > [ 1954.673939] Freezing remaining freezable tasks ...
> > > > [ 1974.681089] Freezing of tasks failed after 20.001 seconds (1 tasks refusing to freeze, wq_busy=0):
> > > > [ 1974.691169] xfsaild/dm-1    D    0  1362      2 0x00000080
> > > > [ 1974.697283] Call Trace:
> > > > [ 1974.700014]  __schedule+0x3be/0x830
> > > > [ 1974.703898]  schedule+0x36/0x80
> > > > [ 1974.707440]  _xfs_log_force+0x143/0x280 [xfs]
> > > > [ 1974.712295]  ? schedule_timeout+0x16b/0x350
> > > > [ 1974.716953]  ? wake_up_q+0x80/0x80
> > > > [ 1974.720752]  ? xfsaild+0x16f/0x770 [xfs]
> > > > [ 1974.725134]  xfs_log_force+0x2c/0x80 [xfs]
> > > > [ 1974.729707]  xfsaild+0x16f/0x770 [xfs]
> > > > [ 1974.733885]  kthread+0x109/0x140
> > > > [ 1974.737480]  ? kthread+0x109/0x140
> > > > [ 1974.741271]  ? xfs_trans_ail_cursor_first+0x90/0x90 [xfs]
> > > > [ 1974.747284]  ? kthread_park+0x60/0x60
> > > > [ 1974.751354]  ret_from_fork+0x25/0x30
> > > > [ 1974.755366] Restarting kernel threads ... done.
> > > > [ 1978.259907] OOM killer enabled.
> > > > [ 1978.263405] Restarting tasks ... done.
> > > > 
> > > > The reason for this failure might be that,
> > > > while the kernel thread xfsaild/dm-1 is waiting for
> > > > xfs-buf/dm-1 to wake it up, however the latter
> > > > has already been frozen, thus xfsaild/dm-1 never
> > > > has a chance to be woken up and get froze. (Although
> > > > the xfsaild/dm-1 remains in TASK_UNINTERRUPTIBLE, which
> > > > is quite similar to 'frozen'.)
> > > 
> > > Should be fixed by this commit in the for-next branch:
> > > 
> > > 0bd89676c4fe xfs: check kthread_should_stop() after the setting of task state
> > > 
> > > That should get merged into 4.15 with the next merge...
> > >
> > I did not quite catch why above commit would fix the issue here,
> > according to
> > https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=0bd89676c4fed53b003025bc4a5200861ac5d8ef
> > it tries to address a race condition between umount and xfsaild on
> > checking the kthread_should_stop() in order not to make
> > xfsaild falling asleep indefinitely.
> 
> Argh, got my threads slightly crossed there.
> 
> > But in our case, the xfsaild is waiting for the xfs-buf to wake
> > it up, and is nothing related to the kthread_should_stop() checking
> > here, did I miss something?
> 
> Similar symptoms - the symptom that was fixed by the commit I
> mentioned was the xfsaild getting stuck in sleeping forever and so
> never seeing the KTHREAD_STOP bit - it was a "set bit vs wakeup"
> race caused by the fact that we didn't reset the state of the
> task correctly after wakeup.
Yes.
> 
> You said:
> 
> >> (Although the xfsaild/dm-1 remains in TASK_UNINTERRUPTIBLE, which
> >> is quite similar to 'frozen'.)
> 
> So from a quick look, it seemed like a similar race condition. I
> missed the *un* part of the task state, though.
> TASK_UNINTERRUPTIBLE implies waiting for IO completion, which is
> what _xfs_log_force() is doing.
> 
Yes.
> SO, follow the other branch of the discussion: hibernation needs to
> freeze filesystems so they can quiesce gracefully before the kernel
> starts shutting down the infrastructure the filesystem relies on...
>
I agree.
Before the filesystem freezing feature is merged into
upstream, wouldn't it be nice if we have some compromise
workaround for such kind of issues: how about treat the
always-sleeping tasks as frozen? They are safe to be
regarded as frozen because they do nothing.
Here's a draft patch to get it done, and it can be
optimized if the direction is acceptible.

diff --git a/kernel/power/process.c b/kernel/power/process.c
index 7381d49a44db..93e123a58558 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -100,8 +100,28 @@ static int try_to_freeze_tasks(bool user_only)
 			read_lock(&tasklist_lock);
 			for_each_process_thread(g, p) {
 				if (p != current && !freezer_should_skip(p)
-				    && freezing(p) && !frozen(p))
-					sched_show_task(p);
+				    && freezing(p) && !frozen(p)) {
+					unsigned long cnt_ctx;
+
+					cnt_ctx = p->nvcsw + p->nivcsw;
+					msleep(MSEC_PER_SEC);
+					/* If the task keeps asleep for 1000 ms
+					 * (actually should be:
+					 * freeze_timeout_msecs+1000ms in theory)
+					 * AKA, there is no context switch for the
+					 * task during this period, we predict this
+					 * task is not likely to do any work in the
+					 * future and we can treat it as frozen.
+					 */
+					if ((p->state & TASK_NORMAL) &&
+					   (cnt_ctx == (p->nvcsw + p->nivcsw))) {
+						pr_err("(%s %c) is sleeping and safe to be treated as frozen\n",
+							p->comm, task_state_to_char(p));
+						todo = 0;
+					} else {
+						sched_show_task(p);
+					}
+				}
 			}
 			read_unlock(&tasklist_lock);
 		}
-- 
2.13.5

Re: [Regression/XFS/PM] Freeze tasks failed in xfsaild

[Michal Hocko]

On Wed 15-11-17 00:39:53, Yu Chen wrote:
[...]
> Before the filesystem freezing feature is merged into
> upstream, wouldn't it be nice if we have some compromise
> workaround for such kind of issues: how about treat the
> always-sleeping tasks as frozen? They are safe to be
> regarded as frozen because they do nothing.
> Here's a draft patch to get it done, and it can be
> optimized if the direction is acceptible.

I do not think this will fly. You have no guarantee those tasks wake up
at any moment after you consider them frozen and declare the system as
quiescent from the userspace POV.

> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 7381d49a44db..93e123a58558 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -100,8 +100,28 @@ static int try_to_freeze_tasks(bool user_only)
>  			read_lock(&tasklist_lock);
>  			for_each_process_thread(g, p) {
>  				if (p != current && !freezer_should_skip(p)
> -				    && freezing(p) && !frozen(p))
> -					sched_show_task(p);
> +				    && freezing(p) && !frozen(p)) {
> +					unsigned long cnt_ctx;
> +
> +					cnt_ctx = p->nvcsw + p->nivcsw;
> +					msleep(MSEC_PER_SEC);
> +					/* If the task keeps asleep for 1000 ms
> +					 * (actually should be:
> +					 * freeze_timeout_msecs+1000ms in theory)
> +					 * AKA, there is no context switch for the
> +					 * task during this period, we predict this
> +					 * task is not likely to do any work in the
> +					 * future and we can treat it as frozen.
> +					 */
> +					if ((p->state & TASK_NORMAL) &&
> +					   (cnt_ctx == (p->nvcsw + p->nivcsw))) {
> +						pr_err("(%s %c) is sleeping and safe to be treated as frozen\n",
> +							p->comm, task_state_to_char(p));
> +						todo = 0;
> +					} else {
> +						sched_show_task(p);
> +					}
> +				}
>  			}
>  			read_unlock(&tasklist_lock);
>  		}
> -- 
> 2.13.5
> 
> 
> 
> 
> 

-- 
Michal Hocko
SUSE Labs