linux-kernel - Re: [PATCH 1/1] hugetlb: fix CONFIG_PADATA dependency for non-SMP system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 5 Feb 2024 14:55:23 +0800
From: Gang Li <gang.li@...ux.dev>
To: Muchun Song <muchun.song@...ux.dev>
Cc: Gang Li <ligang.bdlg@...edance.com>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, Randy Dunlap <rdunlap@...radead.org>,
 kernel test robot <lkp@...el.com>
Subject: Re: [PATCH 1/1] hugetlb: fix CONFIG_PADATA dependency for non-SMP
 system

On 2024/2/4 15:48, Gang Li wrote:
> On 2024/2/4 15:44, Muchun Song wrote:
>> I don't think it is a clear way to fix this. If someone want to
>> use PADATA in a non-SMP system, he should be carefully to handle
>> the non-SMP case himself. I think the better way is to make PADATA
>> handle the non-SMP case, I think it should be easy for it, which
>> could just call ->thread_fn() many times instead of creating many
>> threads in the non-SMP case.
>>
>> Thanks.
>>
> 
> Sounds good, I'll take a look at padata and send a new patch.

1. delete the dependency on SMP

PADATA only depends on workqueue and completion. It works well with !SMP
currently but has no performance benefits. What we can do is make PADATA
handle the non-SMP case more elegantly.

PADATA has two parts: "Running Multithreaded Jobs" and "Running
Serialized Jobs".

"Running Multithreaded Jobs", which hugetlb parallelization relies on
can be easily deparallelize through this patch:

```
@@ -495,7 +495,7 @@ void __init padata_do_multithreaded(struct 
padata_mt_job *job)
        nworks = max(job->size / max(job->min_chunk, job->align), 1ul);
        nworks = min(nworks, job->max_threads);

-      if (nworks == 1) {
+      if (nworks == 1 || !IS_ENABLED(CONFIG_SMP)) {
                /* Single thread, no coordination needed, cut to the 
chase. */
                job->thread_fn(job->start, job->start + job->size, 
job->fn_arg);
                return;
```

However, "Running Serialized Jobs" is more challenging due to its
various workers queuing each other, making it more complex than "Running
Multithreaded Jobs." I am currently in the process of deciphering the
code.

To eliminate kconfig warnings, other methods could be considered:

2. Split hugetlb parallelization into a separate kconfig.
3. Wrap hugetlb parallelization with SMP or PADATA macros (already ruled 
out).
4. Split PADATA into PADATA_SERIALIZED and PADATA_MULTITHREADED (too heavy).

Anyway, this is only FYI. I will continue exploring how to deparallelize
"Running Serialized Jobs."