linux-kernel - Re: [PATCH v6 5/8] padata: downgrade padata_do_multithreaded to serial execution for non-SMP

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: Tue, 5 Mar 2024 11:24:44 +0800
From: Gang Li <gang.li@...ux.dev>
To: Daniel Jordan <daniel.m.jordan@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 David Hildenbrand <david@...hat.com>, David Rientjes <rientjes@...gle.com>,
 Muchun Song <muchun.song@...ux.dev>, Tim Chen <tim.c.chen@...ux.intel.com>,
 Steffen Klassert <steffen.klassert@...unet.com>,
 Jane Chu <jane.chu@...cle.com>, "Paul E . McKenney" <paulmck@...nel.org>,
 Randy Dunlap <rdunlap@...radead.org>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, ligang.bdlg@...edance.com
Subject: Re: [PATCH v6 5/8] padata: downgrade padata_do_multithreaded to
 serial execution for non-SMP

On 2024/2/28 05:26, Daniel Jordan wrote:
> On Thu, Feb 22, 2024 at 10:04:18PM +0800, Gang Li wrote:
>> hugetlb parallelization depends on PADATA, and PADATA depends on SMP.
>>
>> PADATA consists of two distinct functionality: One part is
>> padata_do_multithreaded which disregards order and simply divides
>> tasks into several groups for parallel execution. Hugetlb
>> init parallelization depends on padata_do_multithreaded.
>>
>> The other part is composed of a set of APIs that, while handling data in
>> an out-of-order parallel manner, can eventually return the data with
>> ordered sequence. Currently Only `crypto/pcrypt.c` use them.
>>
>> All users of PADATA of non-SMP case currently only use
>> padata_do_multithreaded. It is easy to implement a serial one in
>> include/linux/padata.h. And it is not necessary to implement another
>> functionality unless the only user of crypto/pcrypt.c does not depend on
>> SMP in the future.
>>
>> Signed-off-by: Gang Li <ligang.bdlg@...edance.com>
>> Tested-by: Paul E. McKenney <paulmck@...nel.org>
>> ---
>>   include/linux/padata.h | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/padata.h b/include/linux/padata.h
>> index 8f418711351bc..0146daf344306 100644
>> --- a/include/linux/padata.h
>> +++ b/include/linux/padata.h
>> @@ -180,10 +180,6 @@ struct padata_instance {
>>   
>>   #ifdef CONFIG_PADATA
>>   extern void __init padata_init(void);
>> -#else
>> -static inline void __init padata_init(void) {}
>> -#endif
>> -
>>   extern struct padata_instance *padata_alloc(const char *name);
>>   extern void padata_free(struct padata_instance *pinst);
>>   extern struct padata_shell *padata_alloc_shell(struct padata_instance *pinst);
>> @@ -194,4 +190,12 @@ 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);
>> +#else
>> +static inline void __init padata_init(void) {}
>> +static inline void __init padata_do_multithreaded(struct padata_mt_job *job)
>> +{
> 
> An early return here for zero-sized jobs is consistent with the
> CONFIG_PADATA version and avoids hugetlb_pages_alloc_boot taking a lock
> and flushing the tlb when there's no work to do.

That's reasonable, thanks!

Since it's single-threaded, the lock isn't contested, but tlb does need
to be treated with caution.

> 
> With that,
> 
> Acked-by: Daniel Jordan <daniel.m.jordan@...cle.com>
> 

And thanks again.

>> +	job->thread_fn(job->start, job->start + job->size, job->fn_arg);
>> +}
>> +#endif
>> +
>>   #endif
>> -- 
>> 2.20.1
>>