Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Zhang\, Qiang" <Qiang.Zhang@windriver.com>,
        "pmladek\@suse.com" <pmladek@suse.com>,
        "tj\@kernel.org" <tj@kernel.org>
Cc:     "akpm\@linux-foundation.org" <akpm@linux-foundation.org>,
        "linux-mm\@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: =?utf-8?B?5Zue5aSNOiDlm57lpI06?= [PATCH v2] kthread_worker:
 re-set CPU affinities if CPU come online
In-Reply-To: <BYAPR11MB2632B18DF7C02B68E758932BFF140@BYAPR11MB2632.namprd11.prod.outlook.com>
References: <20201028073031.4536-1-qiang.zhang@windriver.com> <874kme21nv.fsf@nanos.tec.linutronix.de> <BYAPR11MB263255ED056CED38285FC95BFF170@BYAPR11MB2632.namprd11.prod.outlook.com> <871rhi1z7j.fsf@nanos.tec.linutronix.de> <BYAPR11MB2632B18DF7C02B68E758932BFF140@BYAPR11MB2632.namprd11.prod.outlook.com>
Date:   Thu, 29 Oct 2020 09:27:26 +0100
Message-ID: <874kmdfndd.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Oct 29 2020 at 03:14, Qiang Zhang wrote:
> Really, this patch is not considered that work may be put into the
> queue after the bound CPU is offline.  in addition, when the bound CPU
> goes online again, before restoring the worker's CPU affinity, work
> may be put into the queue.

And how is that supposed to be correct?

> Although int this (powerclamp) way=EF=BC=8Cthat's not a problem, that it =
is
> solved by destroying and creating tasks when the CPU hotplug, in
> addition, when CPU going down , this need call 'cancel_work_sync' func
> in offline callback, this may be blocked long time. these operation is
> expensive.

It does not matter whether it's expensive or not. It's correct and
that's what matters most.

> this patch only just to recover the worker task's affinity when CPU go
> to online again that create by "kthread_create_worker_on_cpu" func ,
> likely per-CPU worker method when CPU hotplug in "workqueue" and
> "io-wq".

I know what this patch just does, but that makes it not any more
correct. It creates a semanticaly ill defined illusion of correctness.

We are not "fixing" a problem by making it work for your particular and
even not explained use case.

The expected semantics of a cpu bound kthread_worker are completely
unclear and undocumented. This needs to be fixed first and once this is
established and agreed on then the gaps in the implementation can be
closed.

Thanks,

        tglx