2020-07-14 20:15:21

by Daniel Jordan

[permalink] [raw]
Subject: [PATCH 0/6] padata cleanups

These cleanups save ~5% of the padata text/data and make it a little
easier to use and develop going forward.

In particular, they pave the way to extend padata's multithreading support to
VFIO, a work-in-progress version of which can be found here:

https://oss.oracle.com/git/gitweb.cgi?p=linux-dmjordan.git;a=shortlog;h=refs/heads/padata-mt-wip-v0.5

Based on v5.8-rc5. As always, feedback is welcome.

Daniel

Daniel Jordan (6):
padata: remove start function
padata: remove stop function
padata: inline single call of pd_setup_cpumasks()
padata: remove effective cpumasks from the instance
padata: fold padata_alloc_possible() into padata_alloc()
padata: remove padata_parallel_queue

Documentation/core-api/padata.rst | 18 +--
crypto/pcrypt.c | 17 +--
include/linux/padata.h | 21 +---
kernel/padata.c | 177 ++++++------------------------
4 files changed, 46 insertions(+), 187 deletions(-)


base-commit: 11ba468877bb23f28956a35e896356252d63c983
--
2.27.0


2020-07-14 20:15:24

by Daniel Jordan

[permalink] [raw]
Subject: [PATCH 6/6] padata: remove padata_parallel_queue

Only its reorder field is actually used now, so remove the struct and
embed @reorder directly in parallel_data.

No functional change, just a cleanup.

Signed-off-by: Daniel Jordan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Steffen Klassert <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
include/linux/padata.h | 15 ++------------
kernel/padata.c | 46 ++++++++++++++++++------------------------
2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/include/linux/padata.h b/include/linux/padata.h
index 070a7d43e8af8..a433f13fc4bf7 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -66,17 +66,6 @@ struct padata_serial_queue {
struct parallel_data *pd;
};

-/**
- * struct padata_parallel_queue - The percpu padata parallel queue
- *
- * @reorder: List to wait for reordering after parallel processing.
- * @num_obj: Number of objects that are processed by this cpu.
- */
-struct padata_parallel_queue {
- struct padata_list reorder;
- atomic_t num_obj;
-};
-
/**
* struct padata_cpumask - The cpumasks for the parallel/serial workers
*
@@ -93,7 +82,7 @@ struct padata_cpumask {
* that depends on the cpumask in use.
*
* @ps: padata_shell object.
- * @pqueue: percpu padata queues used for parallelization.
+ * @reorder_list: percpu reorder lists
* @squeue: percpu padata queues used for serialuzation.
* @refcnt: Number of objects holding a reference on this parallel_data.
* @seq_nr: Sequence number of the parallelized data object.
@@ -105,7 +94,7 @@ struct padata_cpumask {
*/
struct parallel_data {
struct padata_shell *ps;
- struct padata_parallel_queue __percpu *pqueue;
+ struct padata_list __percpu *reorder_list;
struct padata_serial_queue __percpu *squeue;
atomic_t refcnt;
unsigned int seq_nr;
diff --git a/kernel/padata.c b/kernel/padata.c
index 1c0b97891edb8..16cb894dc272b 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -250,13 +250,11 @@ EXPORT_SYMBOL(padata_do_parallel);
static struct padata_priv *padata_find_next(struct parallel_data *pd,
bool remove_object)
{
- struct padata_parallel_queue *next_queue;
struct padata_priv *padata;
struct padata_list *reorder;
int cpu = pd->cpu;

- next_queue = per_cpu_ptr(pd->pqueue, cpu);
- reorder = &next_queue->reorder;
+ reorder = per_cpu_ptr(pd->reorder_list, cpu);

spin_lock(&reorder->lock);
if (list_empty(&reorder->list)) {
@@ -291,7 +289,7 @@ static void padata_reorder(struct parallel_data *pd)
int cb_cpu;
struct padata_priv *padata;
struct padata_serial_queue *squeue;
- struct padata_parallel_queue *next_queue;
+ struct padata_list *reorder;

/*
* We need to ensure that only one cpu can work on dequeueing of
@@ -339,9 +337,8 @@ static void padata_reorder(struct parallel_data *pd)
*/
smp_mb();

- next_queue = per_cpu_ptr(pd->pqueue, pd->cpu);
- if (!list_empty(&next_queue->reorder.list) &&
- padata_find_next(pd, false))
+ reorder = per_cpu_ptr(pd->reorder_list, pd->cpu);
+ if (!list_empty(&reorder->list) && padata_find_next(pd, false))
queue_work(pinst->serial_wq, &pd->reorder_work);
}

@@ -401,17 +398,16 @@ void padata_do_serial(struct padata_priv *padata)
{
struct parallel_data *pd = padata->pd;
int hashed_cpu = padata_cpu_hash(pd, padata->seq_nr);
- struct padata_parallel_queue *pqueue = per_cpu_ptr(pd->pqueue,
- hashed_cpu);
+ struct padata_list *reorder = per_cpu_ptr(pd->reorder_list, hashed_cpu);
struct padata_priv *cur;

- spin_lock(&pqueue->reorder.lock);
+ spin_lock(&reorder->lock);
/* Sort in ascending order of sequence number. */
- list_for_each_entry_reverse(cur, &pqueue->reorder.list, list)
+ list_for_each_entry_reverse(cur, &reorder->list, list)
if (cur->seq_nr < padata->seq_nr)
break;
list_add(&padata->list, &cur->list);
- spin_unlock(&pqueue->reorder.lock);
+ spin_unlock(&reorder->lock);

/*
* Ensure the addition to the reorder list is ordered correctly
@@ -553,17 +549,15 @@ static void padata_init_squeues(struct parallel_data *pd)
}
}

-/* Initialize all percpu queues used by parallel workers */
-static void padata_init_pqueues(struct parallel_data *pd)
+/* Initialize per-CPU reorder lists */
+static void padata_init_reorder_list(struct parallel_data *pd)
{
int cpu;
- struct padata_parallel_queue *pqueue;
+ struct padata_list *list;

for_each_cpu(cpu, pd->cpumask.pcpu) {
- pqueue = per_cpu_ptr(pd->pqueue, cpu);
-
- __padata_list_init(&pqueue->reorder);
- atomic_set(&pqueue->num_obj, 0);
+ list = per_cpu_ptr(pd->reorder_list, cpu);
+ __padata_list_init(list);
}
}

@@ -577,13 +571,13 @@ static struct parallel_data *padata_alloc_pd(struct padata_shell *ps)
if (!pd)
goto err;

- pd->pqueue = alloc_percpu(struct padata_parallel_queue);
- if (!pd->pqueue)
+ pd->reorder_list = alloc_percpu(struct padata_list);
+ if (!pd->reorder_list)
goto err_free_pd;

pd->squeue = alloc_percpu(struct padata_serial_queue);
if (!pd->squeue)
- goto err_free_pqueue;
+ goto err_free_reorder_list;

pd->ps = ps;

@@ -595,7 +589,7 @@ static struct parallel_data *padata_alloc_pd(struct padata_shell *ps)
cpumask_and(pd->cpumask.pcpu, pinst->cpumask.pcpu, cpu_online_mask);
cpumask_and(pd->cpumask.cbcpu, pinst->cpumask.cbcpu, cpu_online_mask);

- padata_init_pqueues(pd);
+ padata_init_reorder_list(pd);
padata_init_squeues(pd);
pd->seq_nr = -1;
atomic_set(&pd->refcnt, 1);
@@ -609,8 +603,8 @@ static struct parallel_data *padata_alloc_pd(struct padata_shell *ps)
free_cpumask_var(pd->cpumask.pcpu);
err_free_squeue:
free_percpu(pd->squeue);
-err_free_pqueue:
- free_percpu(pd->pqueue);
+err_free_reorder_list:
+ free_percpu(pd->reorder_list);
err_free_pd:
kfree(pd);
err:
@@ -621,7 +615,7 @@ static void padata_free_pd(struct parallel_data *pd)
{
free_cpumask_var(pd->cpumask.pcpu);
free_cpumask_var(pd->cpumask.cbcpu);
- free_percpu(pd->pqueue);
+ free_percpu(pd->reorder_list);
free_percpu(pd->squeue);
kfree(pd);
}
--
2.27.0

2020-07-14 20:15:48

by Daniel Jordan

[permalink] [raw]
Subject: [PATCH 2/6] padata: remove stop function

padata_stop() has two callers and is unnecessary in both cases. When
pcrypt calls it before padata_free(), it's being unloaded so there are
no outstanding padata jobs[0]. When __padata_free() calls it, it's
either along the same path or else pcrypt initialization failed, which
of course means there are also no outstanding jobs.

Removing it simplifies padata and saves text.

[0] https://lore.kernel.org/linux-crypto/[email protected]/

Signed-off-by: Daniel Jordan <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Steffen Klassert <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
Documentation/core-api/padata.rst | 16 ++--------------
crypto/pcrypt.c | 12 +++---------
include/linux/padata.h | 1 -
kernel/padata.c | 14 --------------
4 files changed, 5 insertions(+), 38 deletions(-)

diff --git a/Documentation/core-api/padata.rst b/Documentation/core-api/padata.rst
index 0830e5b0e8211..771d50330e5b5 100644
--- a/Documentation/core-api/padata.rst
+++ b/Documentation/core-api/padata.rst
@@ -31,18 +31,7 @@ padata_instance structure for overall control of how jobs are to be run::

'name' simply identifies the instance.

-There are functions for enabling and disabling the instance::
-
- int padata_start(struct padata_instance *pinst);
- void padata_stop(struct padata_instance *pinst);
-
-These functions are setting or clearing the "PADATA_INIT" flag; if that flag is
-not set, other functions will refuse to work. padata_start() returns zero on
-success (flag set) or -EINVAL if the padata cpumask contains no active CPU
-(flag not set). padata_stop() clears the flag and blocks until the padata
-instance is unused.
-
-Finally, complete padata initialization by allocating a padata_shell::
+Then, complete padata initialization by allocating a padata_shell::

struct padata_shell *padata_alloc_shell(struct padata_instance *pinst);

@@ -155,11 +144,10 @@ submitted.
Destroying
----------

-Cleaning up a padata instance predictably involves calling the three free
+Cleaning up a padata instance predictably involves calling the two free
functions that correspond to the allocation in reverse::

void padata_free_shell(struct padata_shell *ps);
- void padata_stop(struct padata_instance *pinst);
void padata_free(struct padata_instance *pinst);

It is the user's responsibility to ensure all outstanding jobs are complete
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 4f5707a3dd1e9..7374dfecaf70f 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -331,12 +331,6 @@ static int pcrypt_init_padata(struct padata_instance **pinst, const char *name)
return ret;
}

-static void pcrypt_fini_padata(struct padata_instance *pinst)
-{
- padata_stop(pinst);
- padata_free(pinst);
-}
-
static struct crypto_template pcrypt_tmpl = {
.name = "pcrypt",
.create = pcrypt_create,
@@ -362,7 +356,7 @@ static int __init pcrypt_init(void)
return crypto_register_template(&pcrypt_tmpl);

err_deinit_pencrypt:
- pcrypt_fini_padata(pencrypt);
+ padata_free(pencrypt);
err_unreg_kset:
kset_unregister(pcrypt_kset);
err:
@@ -373,8 +367,8 @@ static void __exit pcrypt_exit(void)
{
crypto_unregister_template(&pcrypt_tmpl);

- pcrypt_fini_padata(pencrypt);
- pcrypt_fini_padata(pdecrypt);
+ padata_free(pencrypt);
+ padata_free(pdecrypt);

kset_unregister(pcrypt_kset);
}
diff --git a/include/linux/padata.h b/include/linux/padata.h
index 20294cddc7396..7d53208b43daa 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -204,5 +204,4 @@ extern void padata_do_serial(struct padata_priv *padata);
extern void __init padata_do_multithreaded(struct padata_mt_job *job);
extern int padata_set_cpumask(struct padata_instance *pinst, int cpumask_type,
cpumask_var_t cpumask);
-extern void padata_stop(struct padata_instance *pinst);
#endif
diff --git a/kernel/padata.c b/kernel/padata.c
index 9317623166124..8f55e717ba50b 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -789,19 +789,6 @@ int padata_set_cpumask(struct padata_instance *pinst, int cpumask_type,
}
EXPORT_SYMBOL(padata_set_cpumask);

-/**
- * padata_stop - stop the parallel processing
- *
- * @pinst: padata instance to stop
- */
-void padata_stop(struct padata_instance *pinst)
-{
- mutex_lock(&pinst->lock);
- __padata_stop(pinst);
- mutex_unlock(&pinst->lock);
-}
-EXPORT_SYMBOL(padata_stop);
-
#ifdef CONFIG_HOTPLUG_CPU

static int __padata_add_cpu(struct padata_instance *pinst, int cpu)
@@ -883,7 +870,6 @@ static void __padata_free(struct padata_instance *pinst)

WARN_ON(!list_empty(&pinst->pslist));

- padata_stop(pinst);
free_cpumask_var(pinst->rcpumask.cbcpu);
free_cpumask_var(pinst->rcpumask.pcpu);
free_cpumask_var(pinst->cpumask.pcpu);
--
2.27.0