Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Sender: Herbert Xu <herbertx@gmail.com>
Date:   Fri, 15 Sep 2023 18:43:09 +0800
From:   Herbert Xu <herbert@gondor.apana.org.au>
To:     Lu Jialin <lujialin4@huawei.com>
Cc:     Steffen Klassert <steffen.klassert@secunet.com>,
        Daniel Jordan <daniel.m.jordan@oracle.com>,
        "David S . Miller" <davem@davemloft.net>,
        Guo Zihua <guozihua@huawei.com>, linux-crypto@vger.kernel.org
Subject: Re: [PATCH v3] crypto: Fix hungtask for PADATA_RESET
Message-ID: <ZQQ1PYBPm1xqK/hJ@gondor.apana.org.au>
References: <20230904133341.2528440-1-lujialin4@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230904133341.2528440-1-lujialin4@huawei.com>
Precedence: bulk

On Mon, Sep 04, 2023 at 01:33:41PM +0000, Lu Jialin wrote:
> We found a hungtask bug in test_aead_vec_cfg as follows:
> 
> INFO: task cryptomgr_test:391009 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> Call trace:
>  __switch_to+0x98/0xe0
>  __schedule+0x6c4/0xf40
>  schedule+0xd8/0x1b4
>  schedule_timeout+0x474/0x560
>  wait_for_common+0x368/0x4e0
>  wait_for_completion+0x20/0x30
>  wait_for_completion+0x20/0x30
>  test_aead_vec_cfg+0xab4/0xd50
>  test_aead+0x144/0x1f0
>  alg_test_aead+0xd8/0x1e0
>  alg_test+0x634/0x890
>  cryptomgr_test+0x40/0x70
>  kthread+0x1e0/0x220
>  ret_from_fork+0x10/0x18
>  Kernel panic - not syncing: hung_task: blocked tasks
> 
> For padata_do_parallel, when the return err is 0 or -EBUSY, it will call
> wait_for_completion(&wait->completion) in test_aead_vec_cfg. In normal
> case, aead_request_complete() will be called in pcrypt_aead_serial and the
> return err is 0 for padata_do_parallel. But, when pinst->flags is
> PADATA_RESET, the return err is -EBUSY for padata_do_parallel, and it
> won't call aead_request_complete(). Therefore, test_aead_vec_cfg will
> hung at wait_for_completion(&wait->completion), which will cause
> hungtask.
> 
> The problem comes as following:
> (padata_do_parallel)                 |
>     rcu_read_lock_bh();              |
>     err = -EINVAL;                   |   (padata_replace)
>                                      |     pinst->flags |= PADATA_RESET;
>     err = -EBUSY                     |
>     if (pinst->flags & PADATA_RESET) |
>         rcu_read_unlock_bh()         |
>         return err
> 
> In order to resolve the problem, we replace the return err -EBUSY with
> -EAGAIN, which means parallel_data is changing, and the caller should call
> it again.
> 
> v3:
> remove retry and just change the return err.
> v2:
> introduce padata_try_do_parallel() in pcrypt_aead_encrypt and
> pcrypt_aead_decrypt to solve the hungtask.
> 
> Signed-off-by: Lu Jialin <lujialin4@huawei.com>
> Signed-off-by: Guo Zihua <guozihua@huawei.com>
> ---
>  crypto/pcrypt.c | 4 ++++
>  kernel/padata.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt