Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Mon, 16 Oct 2023 12:47:29 +0200
From:   Steffen Klassert <steffen.klassert@secunet.com>
To:     Wang Jinchao <wangjinchao@xfusion.com>
CC:     Daniel Jordan <daniel.m.jordan@oracle.com>,
        <linux-crypto@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <stone.xulei@xfusion.com>
Subject: Re: [RFC/REFACT] Refactoring and significantly reducing code
 complexity
Message-ID: <ZS0UwakMZpKLbsbZ@gauss3.secunet.de>
References: <ZRU/EjubEH/5QLlG@fedora>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZRU/EjubEH/5QLlG@fedora>
Precedence: bulk

Hi,

On Thu, Sep 28, 2023 at 04:53:38PM +0800, Wang Jinchao wrote:
> This is a refactored version with the following main changes:
> 
> - The parallel workqueue no longer uses the WQ_UNBOUND attribute
> - Removal of CPU-related logic, sysfs-related interfaces
> - removal of structures like padata_cpumask, and deletion of parallel_data
> - Using completion to maintain sequencing
> - no longer using lists
> - removing structures like padata_list and padata_serial_queue
> - Removal of padata_do_serial()
> - padata automatically ensures the calling sequence.
> 
> Testing was conducted using ltp's pcrypt_aead01, and the execution time
> comparison between the old and new versions is as follows:
> 
> Old Version:
> real 0m27.451s
> user 0m0.031s
> sys 0m0.260s
> 
> New Version:
> real 0m21.351s
> user 0m0.023s
> sys 0m0.260s
> 
> Signed-off-by: Wang Jinchao <wangjinchao@xfusion.com>
> ---
>  crypto/pcrypt.c        |  34 +-
>  include/linux/padata.h |  95 +----
>  kernel/padata.c        | 815 ++---------------------------------------
>  3 files changed, 43 insertions(+), 901 deletions(-)

...

>  
> -static int pcrypt_sysfs_add(struct padata_instance *pinst, const char *name)
> -{
> -	int ret;
> -
> -	pinst->kobj.kset = pcrypt_kset;
> -	ret = kobject_add(&pinst->kobj, NULL, "%s", name);
> -	if (!ret)
> -		kobject_uevent(&pinst->kobj, KOBJ_ADD);
> -
> -	return ret;
> -}

Users might rely on the existence of that sysfs files,
so removing this might be dangerous.

>  static void padata_serial_worker(struct work_struct *serial_work)
>  {
> -	struct padata_serial_queue *squeue;
> -	struct parallel_data *pd;
> -	LIST_HEAD(local_list);
> -	int cnt;
> -
> -	local_bh_disable();

Note that the networking RX path must run with BHs off.
The call to padata_do_parallel might come with BHs off
and the serialization callback should also called with
BHs off.

Please make sure that the networking usecase continues
to work with that refactorization.

Thanks!