From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Alexander Duyck <alexanderduyck@fb.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ben Segall <bsegall@google.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Ingo Molnar <mingo@redhat.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Josh Triplett <josh@joshtriplett.org>,
        Michal Hocko <mhocko@suse.com>, Nico Pache <npache@redhat.com>,
        Pasha Tatashin <pasha.tatashin@soleen.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        Steve Sistare <steven.sistare@oracle.com>,
        Tejun Heo <tj@kernel.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Vincent Guittot <vincent.guittot@linaro.org>
Cc:     linux-mm@kvack.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
        Daniel Jordan <daniel.m.jordan@oracle.com>
Subject: [RFC 09/16] padata: Use kthreads in do_multithreaded
Date:   Wed,  5 Jan 2022 19:46:49 -0500
Message-Id: <20220106004656.126790-10-daniel.m.jordan@oracle.com>
In-Reply-To: <20220106004656.126790-1-daniel.m.jordan@oracle.com>
References: <20220106004656.126790-1-daniel.m.jordan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3cc6P8bzIKXJ776dvZXiIte/+nGK2kDoWQmIeFRneDGEAWd+NsIv3m4JVqpp?=
 =?us-ascii?Q?fDlGNYQKAAAjemhhCWPvt1msidKPLYYG5JOoclEc1CytFXKBSDbCVMD9jW85?=
 =?us-ascii?Q?DZM/ZTFr1enPK7o+DKPj4auNYeJtUcFTxnHqLLiisDS0LQo64aGUsB9KNSyU?=
 =?us-ascii?Q?vP7Q4Tq+X7dv041WGSmDpBvzTdedUXJs/cO162RwA+10B+pD9DG5F2TAC+fG?=
 =?us-ascii?Q?58ohvhPf1wDDZsClJT6igolE5GU625E6+dafwBH798sSLIc81rqqRDPg8RJn?=
 =?us-ascii?Q?vquf7/lijB1g+3yu3ea8w5lamEXcYc60rBqYBTCo5xtHO7b3QvHxJ2JwSUtV?=
 =?us-ascii?Q?4SY3wP3taZQoYH84O2oiZg2NDXROkb7JJaywFZVkD2TnjEXPc6nNm8A9oHKg?=
 =?us-ascii?Q?45YdSoFnnfv6pRR/GAE49bsV6k/wqD7fFaKKvfAd6TpKdmVnDoqo4j3a9qjz?=
 =?us-ascii?Q?RoW9ie5jHdtGqu+LsacC5Q040MFzt6odeR78bFuIncOR2ve6hyWqoygxSs76?=
 =?us-ascii?Q?C19WBKoUBrMUsiL4kkFcE5klvruqAnB7UAw5i78dYaCZH1Fa4MZbXo9r94Sa?=
 =?us-ascii?Q?JH1wTjH2ZztLKbjIbD9gw7MvIX8IHBdwuXwBEeF2qoKJL7J10HLmzjsotSb2?=
 =?us-ascii?Q?QT21xSA+YtVeTv6nsQJnq1gtIFRRUQqibyy9ygM4gh5V/lIFKfmyT8GG/pTl?=
 =?us-ascii?Q?br1f6hB2lO7NUitoi3nYh3R3n2OvCWxBno+OXNmhLm1KLQe4QYKadXy+rkEO?=
 =?us-ascii?Q?5d/sYEYP71YIu/9t6lUxoQYutt3dqn+jloyzHjGDfWPafRmQA6JL62hbXmgQ?=
 =?us-ascii?Q?puAKoLAcP9H14cT7zl6wEesY6v6FQs+RVcCmlGHuVnTArteQwG8bcIWtSzln?=
 =?us-ascii?Q?90AfWP0f04To+SBcOnTJmbUMXESbAgv5HXG9rbU3g4OtNuTQ5VDVWDMaD07Z?=
 =?us-ascii?Q?65OG0JBpsVdiLbj42rL0vwa3wHU2beBT1kqElrWCmuM8E9bBls+Q6jwxAMmv?=
 =?us-ascii?Q?FsqU7jho84eKBTErqq4FQ0IYJ9/3nF1djP0QVfUsAbQ8SLaNKb8tYhmH84x+?=
 =?us-ascii?Q?pSoPBA3oyWdmLH5CFJS6aPdkRn1qM4QP7WAag1bEIIL2ZZU925hIeYbB0Nxu?=
 =?us-ascii?Q?ZCdIFIqd5eWOGwURLi1M2cw/OgUAFXL1I0iINSYgoiIQyBVAWXHB/4J2jsRx?=
 =?us-ascii?Q?S0BddPtMGw5hVPXbWzsiFqgLTqsWqnTEAIO55Lbz03hnmtYGSOB5IRYHZTe5?=
 =?us-ascii?Q?WsLDT4j5BF1hqnrmENipDkOsy+aZvRXvt05Nrom/hsi0MD0xmssQtVoLaato?=
 =?us-ascii?Q?3FDxZoBQHZLjX7v9wPFhNa0IMKedN9fbqm4rHcmZUrQBXDklDy0yY+t/eDaZ?=
 =?us-ascii?Q?+ryqkHdWnTqB5bNd7i8OS8ejNioCz77SpAkUBKYTGC+QlTnqqOPrEC+XtfPv?=
 =?us-ascii?Q?3fhqcFdnCYhRBHcpjqt4AVbQpSKj5ZQjVwC6rfuUa4ixcLPpt22OOQ+FDDwY?=
 =?us-ascii?Q?1/xjxCTknFBx8ErGfLhKLq1dRBNoWucFhLXVQeYoDGnbdKvbj/6y14TTnf9Y?=
 =?us-ascii?Q?WshF74Lttl/xwAFUGgtbeyi4vYX+nZpR4TBGVfDrHIDUe8pZp8XZbrQ5rHiP?=
 =?us-ascii?Q?MfpVwkCqSoYeUI9DsqB1zF8dz5RbK6tvDtRprCkY8qqMqNiyoMGogKeeJOby?=
 =?us-ascii?Q?uEfWnB2c/VjxJD62shlK8s3V344=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2ec127d2-81f6-444d-be20-08d9d0ae22e5
X-MS-Exchange-CrossTenant-AuthSource: PH7PR10MB5698.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jan 2022 00:47:37.9587
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 7uuyvnuuphMJMrFj9O5WmV1dqNPj4teqeIW4hOSfnHQa3uEYGM5BwirACUnD3THYK3DdJs3fRHiyFjzeCjxuumcUiriI7xfivbFvQshh54g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4422
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10218 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 mlxscore=0
 malwarescore=0 suspectscore=0 mlxlogscore=999 spamscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2112160000
 definitions=main-2201060001
X-Proofpoint-ORIG-GUID: Gg9wq4R0jhnrPL-28Hs0olXvb0H3RzcU
X-Proofpoint-GUID: Gg9wq4R0jhnrPL-28Hs0olXvb0H3RzcU
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Unbound kworkers will soon not be ideal for multithreaded jobs because
helpers will inherit the resource controls of the main thread, but
changing these controls (e.g. CPU affinity) might conflict with those
that kworkers already have in place.  While the changes are only
temporary, it seems like a layering violation to mess with kworkers this
way, and undoing the settings might fail or add latency for future
works.

Use kthreads instead, which have none of these issues.

Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
---
 kernel/padata.c | 47 ++++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/kernel/padata.c b/kernel/padata.c
index b458deb17121..00509c83e356 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -29,6 +29,7 @@
 #include <linux/cpumask.h>
 #include <linux/err.h>
 #include <linux/cpu.h>
+#include <linux/kthread.h>
 #include <linux/list_sort.h>
 #include <linux/padata.h>
 #include <linux/mutex.h>
@@ -37,8 +38,6 @@
 #include <linux/sysfs.h>
 #include <linux/rcupdate.h>
 
-#define	PADATA_WORK_ONSTACK	1	/* Work's memory is on stack */
-
 struct padata_work {
 	struct work_struct	pw_work;
 	struct list_head	pw_list;  /* padata_free_works linkage */
@@ -70,7 +69,6 @@ struct padata_mt_job_state {
 };
 
 static void padata_free_pd(struct parallel_data *pd);
-static void padata_mt_helper(struct work_struct *work);
 
 static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
 {
@@ -108,17 +106,7 @@ static struct padata_work *padata_work_alloc(void)
 	return pw;
 }
 
-static void padata_work_init(struct padata_work *pw, work_func_t work_fn,
-			     void *data, int flags)
-{
-	if (flags & PADATA_WORK_ONSTACK)
-		INIT_WORK_ONSTACK(&pw->pw_work, work_fn);
-	else
-		INIT_WORK(&pw->pw_work, work_fn);
-	pw->pw_data = data;
-}
-
-static int padata_work_alloc_mt(int nworks, void *data, struct list_head *head)
+static int padata_work_alloc_mt(int nworks, struct list_head *head)
 {
 	int i;
 
@@ -129,7 +117,6 @@ static int padata_work_alloc_mt(int nworks, void *data, struct list_head *head)
 
 		if (!pw)
 			break;
-		padata_work_init(pw, padata_mt_helper, data, 0);
 		list_add(&pw->pw_list, head);
 	}
 	spin_unlock(&padata_works_lock);
@@ -234,7 +221,8 @@ int padata_do_parallel(struct padata_shell *ps,
 	rcu_read_unlock_bh();
 
 	if (pw) {
-		padata_work_init(pw, padata_parallel_worker, padata, 0);
+		INIT_WORK(&pw->pw_work, padata_parallel_worker);
+		pw->pw_data = padata;
 		queue_work(pinst->parallel_wq, &pw->pw_work);
 	} else {
 		/* Maximum works limit exceeded, run in the current task. */
@@ -449,9 +437,9 @@ static int padata_setup_cpumasks(struct padata_instance *pinst)
 	return err;
 }
 
-static void padata_mt_helper(struct work_struct *w)
+static int padata_mt_helper(void *__pw)
 {
-	struct padata_work *pw = container_of(w, struct padata_work, pw_work);
+	struct padata_work *pw = __pw;
 	struct padata_mt_job_state *ps = pw->pw_data;
 	struct padata_mt_job *job = ps->job;
 	bool done;
@@ -500,6 +488,8 @@ static void padata_mt_helper(struct work_struct *w)
 
 	if (done)
 		complete(&ps->completion);
+
+	return 0;
 }
 
 static int padata_error_cmp(void *unused, const struct list_head *a,
@@ -593,7 +583,7 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 	lockdep_init_map(&ps.lockdep_map, map_name, key, 0);
 	INIT_LIST_HEAD(&ps.failed_works);
 	ps.job		  = job;
-	ps.nworks	  = padata_work_alloc_mt(nworks, &ps, &works);
+	ps.nworks	  = padata_work_alloc_mt(nworks, &works);
 	ps.nworks_fini	  = 0;
 	ps.error	  = 0;
 	ps.position	  = job->start;
@@ -612,13 +602,21 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 	lock_map_acquire(&ps.lockdep_map);
 	lock_map_release(&ps.lockdep_map);
 
-	list_for_each_entry(pw, &works, pw_list)
-		queue_work(system_unbound_wq, &pw->pw_work);
+	list_for_each_entry(pw, &works, pw_list) {
+		struct task_struct *task;
+
+		pw->pw_data = &ps;
+		task = kthread_create(padata_mt_helper, pw, "padata");
+		if (IS_ERR(task))
+			--ps.nworks;
+		else
+			wake_up_process(task);
+	}
 
-	/* Use the current thread, which saves starting a workqueue worker. */
-	padata_work_init(&my_work, padata_mt_helper, &ps, PADATA_WORK_ONSTACK);
+	/* Use the current task, which saves starting a kthread. */
+	my_work.pw_data = &ps;
 	INIT_LIST_HEAD(&my_work.pw_list);
-	padata_mt_helper(&my_work.pw_work);
+	padata_mt_helper(&my_work);
 
 	/* Wait for all the helpers to finish. */
 	wait_for_completion(&ps.completion);
@@ -626,7 +624,6 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 	if (ps.error && job->undo_fn)
 		padata_undo(&ps, &works, &my_work);
 
-	destroy_work_on_stack(&my_work.pw_work);
 	padata_works_free(&works);
 	return ps.error;
 }
-- 
2.34.1