Re: [PATCH 5.15 00/23] 5.15.160-rc1 review

NeilBrown
2024-05-29T21:00:09Z

On Wed, 29 May 2024, Jon Hunter wrote:
> On 29/05/2024 00:42, NeilBrown wrote:
> > On Wed, 29 May 2024, NeilBrown wrote:
> >>
> >> We probably just need to add "| TASK_FREEZABLE" in one or two places.
> >> I'll post a patch for testing in a little while.
> >
> > There is no TASK_FREEZABLE before v6.1.
> > This isn't due to a missed backport. It is simply because of differences
> > in the freezer in older kernels.
> >
> > Please test this patch.
> >
> > Thanks,
> > NeilBrown
> >
> > From 416bd6ae9a598e64931d34b76aa58f39b11841cd Mon Sep 17 00:00:00 2001
> > From: NeilBrown <neilb@suse.de>
> > Date: Wed, 29 May 2024 09:38:22 +1000
> > Subject: [PATCH] sunrpc: exclude from freezer when waiting for requests:
> >
> > Prior to v6.1, the freezer will only wake a kernel thread from an
> > uninterruptible sleep. Since we changed svc_get_next_xprt() to use and
> > IDLE sleep the freezer cannot wake it. we need to tell the freezer to
> > ignore it instead.
> >
> > Fixes: 9b8a8e5e8129 ("nfsd: don't allow nfsd threads to be signalled.")
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > net/sunrpc/svc_xprt.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > index b19592673eef..12e9293bd12b 100644
> > --- a/net/sunrpc/svc_xprt.c
> > +++ b/net/sunrpc/svc_xprt.c
> > @@ -764,10 +764,12 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp, long timeout)
> > clear_bit(RQ_BUSY, &rqstp->rq_flags);
> > smp_mb__after_atomic();
> >
> > + freezer_do_not_count();
> > if (likely(rqst_should_sleep(rqstp)))
> > time_left = schedule_timeout(timeout);
> > else
> > __set_current_state(TASK_RUNNING);
> > + freezer_count();
> >
> > try_to_freeze();
> >
>
>
> Thanks. I gave this a try on top of v5.15.160-rc1, but I am still seeing
> the following and the board hangs ...
>
> Freezing of tasks failed after 20.004 seconds (1 tasks refusing to freeze, wq_busy=0):
>
> So unfortunately this does not fix it :-(

Thanks for testing.
I can only guess that you had an active NFSv4.1 mount and that the
callback thread was causing problems. Please try this. I also changed
to use freezable_schedule* which seems like a better interface to do the
same thing.

If this doesn't fix it, we'll probably need to ask someone who remembers
the old freezer code.

Thanks,
NeilBrown

From 518f0c1150f988b3fe8e5e0d053a25c3aa6c7d44 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Wed, 29 May 2024 09:38:22 +1000
Subject: [PATCH] sunrpc: exclude from freezer when waiting for requests:

Prior to v6.1, the freezer will only wake a kernel thread from an
uninterruptible sleep. Since we changed svc_get_next_xprt() to use and
IDLE sleep the freezer cannot wake it. we need to tell the freezer to
ignore it instead.

To make this work with only upstream requests we would need
Commit f5d39b020809 ("freezer,sched: Rewrite core freezer logic")
which allows non-interruptible sleeps to be woken by the freezer.

Fixes: 9b8a8e5e8129 ("nfsd: don't allow nfsd threads to be signalled.")
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfs/callback.c | 2 +-
fs/nfsd/nfs4proc.c | 3 ++-
net/sunrpc/svc_xprt.c | 4 ++--
3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 46a0a2d6962e..8fe143cad4a2 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -124,7 +124,7 @@ nfs41_callback_svc(void *vrqstp)
} else {
spin_unlock_bh(&serv->sv_cb_lock);
if (!kthread_should_stop())
- schedule();
+ freezable_schedule();
finish_wait(&serv->sv_cb_waitq, &wq);
}
}
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 6779291efca9..e0ff2212866a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -38,6 +38,7 @@
#include <linux/slab.h>
#include <linux/kthread.h>
#include <linux/namei.h>
+#include <linux/freezer.h>

#include <linux/sunrpc/addr.h>
#include <linux/nfs_ssc.h>
@@ -1322,7 +1323,7 @@ static __be32 nfsd4_ssc_setup_dul(struct nfsd_net *nn, char *ipaddr,

/* allow 20secs for mount/unmount for now - revisit */
if (kthread_should_stop() ||
- (schedule_timeout(20*HZ) == 0)) {
+ (freezable_schedule_timeout(20*HZ) == 0)) {
finish_wait(&nn->nfsd_ssc_waitq, &wait);
kfree(work);
return nfserr_eagain;
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index b19592673eef..3cf53e3140a5 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -705,7 +705,7 @@ static int svc_alloc_arg(struct svc_rqst *rqstp)
set_current_state(TASK_RUNNING);
return -EINTR;
}
- schedule_timeout(msecs_to_jiffies(500));
+ freezable_schedule_timeout(msecs_to_jiffies(500));
}
rqstp->rq_page_end = &rqstp->rq_pages[pages];
rqstp->rq_pages[pages] = NULL; /* this might be seen in nfsd_splice_actor() */
@@ -765,7 +765,7 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp, long timeout)
smp_mb__after_atomic();

if (likely(rqst_should_sleep(rqstp)))
- time_left = schedule_timeout(timeout);
+ time_left = freezable_schedule_timeout(timeout);
else
__set_current_state(TASK_RUNNING);

--
2.44.0