Skip to contentSkip to conversation

padata: Simplify sysfs cpumask and sequencing logic

RFC Wang Jinchao 2 participants 3 messages Oct 12, 2023 – Oct 26, 2023 Crypto API

lore
Share & tools
lore
#0 lore raw

Hi,

I've identified several potential optimizations for padata. I'd appreciate it if you could take a look at my ideas to see if they are feasible.

Utilizing the WQ_SYSFS from workqueue to support sysfs
======================================================

Padata relies on workqueue, and since workqueue has already implemented support for cpumask through WQ_SYSFS, we can reuse this functionality and avoid redundant implementation.

Using completion to ensure the sequencing of the 'serial()'
===========================================================

In the current implementation, to ensure the sequencing of 'serial()', we've used seq_nr, reorder_list, padata_serial_queue, reorder_work... which has made the logic quite complex. These operations can be simplified by using 'completion'. Specifically:
1. in padata_do_parallel()
1. init_completion(parallel_done) **before** queue_work
2. queue_work(serial_work)
2. in padata_parallel_worker
1. complete(parallel_done) **after** parallel(padata)
3. in padata_serial_worker
1. wait_for_completion(parallel_done) **before** serial(padata)

Here's a simplified code snippet:

```c
struct padata_priv {
	struct completion parallel_done;
	struct work_struct	parallel_work;
	struct work_struct	serial_work;
	void   (*parallel)(struct padata_priv *padata);
	void   (*serial)(struct padata_priv *padata);
}

void padata_do_parallel(struct padata_priv *padata)
{
    ...
    init_completion(&padata->parallel_done);
	queue_work(pinst->serial_wq, &padata->serial_work);
	queue_work(pinst->parallel_wq, &padata->parallel_work);
    ...
}

static void padata_parallel_worker(struct work_struct *parallel_work)
{
	struct padata_priv *padata =
		container_of(parallel_work, struct padata_priv, parallel_work);
	padata->parallel(padata);
	// notify serial_worker to do serial()
	complete(&padata->parallel_done);
}

static void padata_serial_worker(struct work_struct *serial_work)
{
	struct padata_priv *padata =
		container_of(serial_work, struct padata_priv, serial_work);
	wait_for_completion(&padata->parallel_done);
	padata->serial(padata);
}
```
#1 lore raw

Hello,

Quoted 7 lines · Wang Jinchao

Utilizing the WQ_SYSFS from workqueue to support sysfs
======================================================

Padata relies on workqueue, and since workqueue has already implemented support for cpumask through WQ_SYSFS, we can reuse this functionality and avoid redundant implementation.

Keeping the existing padata sysfs paths seems best, as mentioned in the other thread.

Quoted 2 lines

Using completion to ensure the sequencing of the 'serial()'
===========================================================

I responded in the other RFC thread.

Thanks.

#2 lore raw
Quoted 6 lines · Daniel Jordan

Hello,

Quoted 3 lines · Wang Jinchao

Utilizing the WQ_SYSFS from workqueue to support sysfs
======================================================

Quoted 7 lines
Quoted 4 lines

Padata relies on workqueue, and since workqueue has already implemented support for cpumask through WQ_SYSFS, we can reuse this functionality and avoid redundant implementation.

Keeping the existing padata sysfs paths seems best, as mentioned in the other thread.

Symlinks can serve both purposes, keeping the original path and simplifying the code. I will incorporate this in the subsequent patches.

Quoted 8 lines
Quoted 3 lines

Using completion to ensure the sequencing of the 'serial()'
===========================================================

I responded in the other RFC thread.

Thanks.

The extensive changes in RFC v1 indeed made it challenging to read. Thank you very much for your patience. In RFC v2, I only wrote the core logic instead of lengthy patches,making it a true RFC. I've carefully considered your and Steffen's feedback, which will be addressed in RFC v3.

Thanks.