regressions.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Patrick Schaaf <bof@bof.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org, regressions@lists.linux.dev,
	"Igor Lončarević" <anubis@igorloncarevic.com>
Subject: Re: stable 5.4.135 REGRESSION / once-a-day WARNING: at kthread_is_per_cpu+0x1c/0x20
Date: Tue, 31 Aug 2021 16:57:11 +0200	[thread overview]
Message-ID: <CAJ26g5Toxq-WaRFvP9V-dNL0GXx+w5PTakcQw2stW1HteUhS3w@mail.gmail.com> (raw)
In-Reply-To: <YS3TpF8B5TA2mGFr@hirez.programming.kicks-ass.net>

On Tue, Aug 31, 2021 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> 3a7956e25e1d ("kthread: Fix PF_KTHREAD vs to_kthread() race") munged
> into 5.4.135
>
> Never even seen a compiler, please tests.

Thanks a lot, Peter. With the addition of a little semicolon in one
place, I successfully built 5.4.143 with the patch applied, in both my
host and vm configs.

I now have two machines with identical workloads (webserver, database)
running, one with unpatched 5.4.143, the other with 5.4.143 and that
patch. Expectation is to see the WARNING on the former, but not on the
latter. Will report in a day or three about the outcome.

regards
  Patrick

Tested-By: Patrick Schaaf <bof@bof.de>

diff --git a/kernel/kthread.c b/kernel/kthread.c
index b2bac5d929d2..22750a8af83e 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -76,6 +76,25 @@ static inline struct kthread *to_kthread(struct
task_struct *k)
        return (__force void *)k->set_child_tid;
 }

+/*
+ * Variant of to_kthread() that doesn't assume @p is a kthread.
+ *
+ * Per construction; when:
+ *
+ *   (p->flags & PF_KTHREAD) && p->set_child_tid
+ *
+ * the task is both a kthread and struct kthread is persistent. However
+ * PF_KTHREAD on it's own is not, kernel_thread() can exec() (See umh.c and
+ * begin_new_exec()).
+ */
+static inline struct kthread *__to_kthread(struct task_struct *p)
+{
+       void *kthread = (__force void *)p->set_child_tid;
+       if (kthread && !(p->flags & PF_KTHREAD))
+               kthread = NULL;
+       return kthread;
+}
+
 void free_kthread_struct(struct task_struct *k)
 {
        struct kthread *kthread;
@@ -176,10 +195,11 @@ void *kthread_data(struct task_struct *task)
  */
 void *kthread_probe_data(struct task_struct *task)
 {
-       struct kthread *kthread = to_kthread(task);
+       struct kthread *kthread = __to_kthread(task);
        void *data = NULL;

-       probe_kernel_read(&data, &kthread->data, sizeof(data));
+       if (kthread)
+               probe_kernel_read(&data, &kthread->data, sizeof(data));
        return data;
 }

@@ -490,9 +510,9 @@ void kthread_set_per_cpu(struct task_struct *k, int cpu)
        set_bit(KTHREAD_IS_PER_CPU, &kthread->flags);
 }

-bool kthread_is_per_cpu(struct task_struct *k)
+bool kthread_is_per_cpu(struct task_struct *p)
 {
-       struct kthread *kthread = to_kthread(k);
+       struct kthread *kthread = __to_kthread(p);
        if (!kthread)
                return false;

@@ -1272,11 +1292,9 @@ EXPORT_SYMBOL(kthread_destroy_worker);
  */
 void kthread_associate_blkcg(struct cgroup_subsys_state *css)
 {
-       struct kthread *kthread;
+       struct kthread *kthread = __to_kthread(current);
+

-       if (!(current->flags & PF_KTHREAD))
-               return;
-       kthread = to_kthread(current);
        if (!kthread)
                return;

@@ -1298,13 +1316,10 @@ EXPORT_SYMBOL(kthread_associate_blkcg);
  */
 struct cgroup_subsys_state *kthread_blkcg(void)
 {
-       struct kthread *kthread;
+       struct kthread *kthread = __to_kthread(current);

-       if (current->flags & PF_KTHREAD) {
-               kthread = to_kthread(current);
-               if (kthread)
-                       return kthread->blkcg_css;
-       }
+       if (kthread)
+               return kthread->blkcg_css;
        return NULL;
 }
 EXPORT_SYMBOL(kthread_blkcg);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 74cb20f32f72..87d9fad9d01d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7301,7 +7301,7 @@ int can_migrate_task(struct task_struct *p,
struct lb_env *env)
                return 0;

        /* Disregard pcpu kthreads; they are where they need to be. */
-       if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+       if (kthread_is_per_cpu(p))
                return 0;

        if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {

  reply	other threads:[~2021-08-31 14:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 17:02 bof
2021-07-28 17:45 ` Greg KH
2021-07-28 17:51   ` Patrick Schaaf
2021-08-31  4:46   ` Patrick Schaaf
2021-08-31  7:00     ` Peter Zijlstra
2021-08-31 14:57       ` Patrick Schaaf [this message]
2021-07-28 17:40 bof

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ26g5Toxq-WaRFvP9V-dNL0GXx+w5PTakcQw2stW1HteUhS3w@mail.gmail.com \
    --to=bof@bof.de \
    --cc=anubis@igorloncarevic.com \
    --cc=peterz@infradead.org \
    --cc=regressions@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --subject='Re: stable 5.4.135 REGRESSION / once-a-day WARNING: at kthread_is_per_cpu+0x1c/0x20' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).