Received-SPF: pass (google.com: domain of linux-kernel+bounces-91609-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <632e7fa2-1c46-4b78-a407-9e6b9c410ea4@linux.dev>
Date: Tue, 5 Mar 2024 11:24:44 +0800
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v6 5/8] padata: downgrade padata_do_multithreaded to
 serial execution for non-SMP
Content-Language: en-US
To: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
 David Hildenbrand <david@redhat.com>, David Rientjes <rientjes@google.com>,
 Muchun Song <muchun.song@linux.dev>, Tim Chen <tim.c.chen@linux.intel.com>,
 Steffen Klassert <steffen.klassert@secunet.com>,
 Jane Chu <jane.chu@oracle.com>, "Paul E . McKenney" <paulmck@kernel.org>,
 Randy Dunlap <rdunlap@infradead.org>, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org, ligang.bdlg@bytedance.com
References: <20240222140422.393911-1-gang.li@linux.dev>
 <20240222140422.393911-6-gang.li@linux.dev>
 <e44cv3c7lafc2a5p4wkhxjaipq4hgclzuceignzzp37kl4l2pj@t7bgio7s7p4q>
From: Gang Li <gang.li@linux.dev>
In-Reply-To: <e44cv3c7lafc2a5p4wkhxjaipq4hgclzuceignzzp37kl4l2pj@t7bgio7s7p4q>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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@bytedance.com>
>> Tested-by: Paul E. McKenney <paulmck@kernel.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@oracle.com>
> 

And thanks again.

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