| Message ID | 20220818214049.1519544-1-lalithkraj@google.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | ext4: Make ext4_lazyinit_thread freezable. | expand |
On Thu, Aug 18, 2022 at 09:40:49PM +0000, Lalith Rajendran wrote: > ext4_lazyinit_thread is not set freezable. Hence when the thread calls > try_to_freeze it doesn't freeze during suspend and continues to send > requests to the storage during suspend, resulting in suspend failures. Maybe we should just make suspend freeze all the filesystems in order? https://lore.kernel.org/linux-fsdevel/20210417001026.23858-1-mcgrof@kernel.org/ --D > Signed-off-by: Lalith Rajendran <lalithkraj@google.com> > --- > fs/ext4/super.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 9a66abcca1a85..d77e0904a1327 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -3767,6 +3767,7 @@ static int ext4_lazyinit_thread(void *arg) > unsigned long next_wakeup, cur; > > BUG_ON(NULL == eli); > + set_freezable(); > > cont_thread: > while (true) { > -- > 2.31.0 >
On Tue, Aug 23, 2022 at 08:37:43AM -0700, Lalith Rajendran wrote: > Thank you for the comment. > Can you provide more details on how > https://lore.kernel.org/linux-fsdevel/20210417001026.23858-1-mcgrof@kernel.org/ > is > related to your comment to freeze all filesystems in order? This all got started a long time ago[0] when I noticed that suspend often fails on my laptop because the kthread freezer will shut down some of XFS' low level background kernel threads before higher level background threads have been put to sleep. In this case, suspend kills the buffer IO completion thread before the XFS log worker, which means the log worker stalls trying to write the log contents back to disk: [0] https://lore.kernel.org/linux-xfs/20170203010401.GR9134@birch.djwong.org/ So I then proposed a patch[1] to mark the buffer completion workqueue as non-freezeable, which was rejected because while that will bandaid the suspend problem, it'll create bigger problems with hibernation because the log and buffer workers can keep running while hibernate tries to write the state of those threads out to disk: [1] https://lore.kernel.org/linux-xfs/20170327204611.GA4864@birch.djwong.org/ That came with a suggestion that the power management code should freeze the filesystems before trying to put kthreads to sleep. User programs will go to sleep trying to get write access, and all fs background threads will already be idle. Luis Chamberlain offered to pick up that work[2] back in 2017 to figure out how to freeze filesystems prior to suspend. His first attempt simply walked the supers list in reverse order, and drew quite a few comments. The second version[3] improved on that. [2] https://lore.kernel.org/all/20171003185313.1017-1-mcgrof@kernel.org/T/#u [3] https://lore.kernel.org/all/20171129232356.28296-1-mcgrof@kernel.org/T/#u Four years went by (I don't blame Luis; I've not had time to get back to this either) and his most recent posting[4] I think tried to restart the discussion. [4] https://lore.kernel.org/linux-fsdevel/20210417001026.23858-1-mcgrof@kernel.org/ So that's where things stand now. I've idly wondered if a better approach would be to hook filesystems into the device model so that filesystems could watch for suspend notifications propagating down the device tree and turn that into a fsfreeze, but as I mentioned, I've had no time to figure out how that would really work since I'm not that familiar with how power management works in the kernel. I don't have a clue where you'd attach a network filesystem. --D > Thanks, > Lalith > > On Thu, Aug 18, 2022 at 5:08 PM Darrick J. Wong <djwong@kernel.org> wrote: > > > On Thu, Aug 18, 2022 at 09:40:49PM +0000, Lalith Rajendran wrote: > > > ext4_lazyinit_thread is not set freezable. Hence when the thread calls > > > try_to_freeze it doesn't freeze during suspend and continues to send > > > requests to the storage during suspend, resulting in suspend failures. > > > > Maybe we should just make suspend freeze all the filesystems in order? > > > > > > https://lore.kernel.org/linux-fsdevel/20210417001026.23858-1-mcgrof@kernel.org/ > > > > --D > > > > > Signed-off-by: Lalith Rajendran <lalithkraj@google.com> > > > --- > > > fs/ext4/super.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > > index 9a66abcca1a85..d77e0904a1327 100644 > > > --- a/fs/ext4/super.c > > > +++ b/fs/ext4/super.c > > > @@ -3767,6 +3767,7 @@ static int ext4_lazyinit_thread(void *arg) > > > unsigned long next_wakeup, cur; > > > > > > BUG_ON(NULL == eli); > > > + set_freezable(); > > > > > > cont_thread: > > > while (true) { > > > -- > > > 2.31.0 > > > > >
On Thu, 18 Aug 2022 21:40:49 +0000, Lalith Rajendran wrote: > ext4_lazyinit_thread is not set freezable. Hence when the thread calls > try_to_freeze it doesn't freeze during suspend and continues to send > requests to the storage during suspend, resulting in suspend failures. > > Applied, thanks! [1/1] ext4: Make ext4_lazyinit_thread freezable. commit: 5442d7d5073d1500c542ac0f2c881b738b1ef3a5 Best regards,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 9a66abcca1a85..d77e0904a1327 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3767,6 +3767,7 @@ static int ext4_lazyinit_thread(void *arg) unsigned long next_wakeup, cur; BUG_ON(NULL == eli); + set_freezable(); cont_thread: while (true) {
ext4_lazyinit_thread is not set freezable. Hence when the thread calls try_to_freeze it doesn't freeze during suspend and continues to send requests to the storage during suspend, resulting in suspend failures. Signed-off-by: Lalith Rajendran <lalithkraj@google.com> --- fs/ext4/super.c | 1 + 1 file changed, 1 insertion(+)