Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Fri, 13 Oct 2023 11:22:03 -0400
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Wang Jinchao <wangjinchao@xfusion.com>
Cc:     Steffen Klassert <steffen.klassert@secunet.com>,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
        stone.xulei@xfusion.com
Subject: Re: [PATCH v3] Fixes: 07928d9bfc81 ("padata: Remove broken queue
 flushing")
Message-ID: <tgveetqzkm4if3kkq5mzhgfetbm4nqe5coegphdtyye3xul6r4@5cr6uvfrinow>
References: <ZSDWAcUxXcwD4YUZ@fedora>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZSDWAcUxXcwD4YUZ@fedora>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?NzZBC3nkU9vxCCLAAEw7HWMkYFQlworLiKwbtp0v+BLoBubrcFgKGyD49Cs8?=
 =?us-ascii?Q?z63b8GUVeQtyLLKNiuMvGd7fTK0Lof+64Oz/7ui7iBMOtxN0eR2H8S7XVNXt?=
 =?us-ascii?Q?6V9TpYu/12uqEcHyBFZNWzARi9LwHU5LaCiUTAd1MQVLkntELZVzcAUgYpP0?=
 =?us-ascii?Q?oB9wPmV9TkbOjYrPpdgtj9w4IpzwF1SZZezDI1X89HZjuIfGL4R70W2EkP//?=
 =?us-ascii?Q?jWYeLNLrX1AmO7QuxLZ2KnjteqCwzE+gsNReaO5U2+rcictxdruQRdD4C1Px?=
 =?us-ascii?Q?ryo8+NtObi7ZOhltSOkB5679FZlq9jYz0b4yNpXzvhNkNTyl9zHMsjXSa0Ow?=
 =?us-ascii?Q?soUYvtJGE6lTgaWndGGjJYuPVHuJgCaDIbZZ5uAq8/Bu66o/KuyKNngw2aAx?=
 =?us-ascii?Q?QOfMnhIPG2guyfSq7iNTtB/Mw3UKVPIwWlnyhXPHw/9JMef523mjQdkVW68r?=
 =?us-ascii?Q?iKDmtpWXyhDSWBuk597vpbERvOXq5W8FXxMzp4u+H5ZASIhliJcja8zbo4LE?=
 =?us-ascii?Q?QCoF+q55ppyMgL7VkyzRLfUdUqZANUnobEiKle8IjH7wFaIIk0L/EGPHFMDp?=
 =?us-ascii?Q?KGwWGvAK9X1X3Tz1n2ibHd0YFV9atHkYEs3F806o/4DfWAYH5FFA1kX8Rg5+?=
 =?us-ascii?Q?Og6SrHPJqOy2/stS+VPuldpI0pR8OBl91SV81+KG9rClRWPhHg6YmHCF2gHw?=
 =?us-ascii?Q?maUzUbFa3l3ShOVgIAWvsxmmmVUWLIxqNFYs2x6SFr85s8LTEHP06NAaXt8o?=
 =?us-ascii?Q?CmiVCmYnK0EusvE5jtOdIJJzPUBb/qKTIiDeB0j7iBTLjsJtuu6tvdHKbyaw?=
 =?us-ascii?Q?GTvgUoFNiliYS4VYbg2/xMNiwO9J7XOwK448ouSxZLf75Hr/Zi5EPDGyw7so?=
 =?us-ascii?Q?raIq6w/xqHDtPOm8OBuyaJl8hJcuFrQY2vfrPjTe4aRaEuLrih3cNeqk8dUF?=
 =?us-ascii?Q?AYKb/iHDhvKAKlbujss6DqAXikXlBxObWaDh1O0ZZOQva84UaMmUksaE9RK8?=
 =?us-ascii?Q?tZql9hVZGpXbi9BQ4rcNyZPyHaGyrLqDO5r4VZWNdf9TQCIriYdlhwkIID0C?=
 =?us-ascii?Q?T5VaqSFm4Zg+gIZpZTP/U70FRGDsohnrpQVbaBKaAgXQXQkkekG616xk2VAm?=
 =?us-ascii?Q?n1eXuximIX8c/xy5A8oI3f0dox9NXjrZd0mzT9EmeJBaupx/qOdi33Mxg8gd?=
 =?us-ascii?Q?s+kYO1T3E/AQp7P78OcRM8sGyYxNiZK58+RuAiZjne2RB+KtUvgTWm1Bhj++?=
 =?us-ascii?Q?K3LlyUr8HYsWcNsIldCbrc/W/qej5cV4CijSjH3adU0YILlDMY4c4NWvOm+j?=
 =?us-ascii?Q?pMinHEN5NGvAuadA+riUR5W3+aGC/oQ3w0ZMvkTvJF+j87E4u7kqKylskyAB?=
 =?us-ascii?Q?seYZuRbkBavuQDXguqcfd3dZMvEKGTgJOovXVMiLcozFlSMeAzT5NhYMyRBf?=
 =?us-ascii?Q?ltuL3xIzTMwzANban8BKdtgkbLU/jHj4dcfFOh26Ka5zinHfjvi6buthvZZl?=
 =?us-ascii?Q?TWbJjKm+UDiV4Ukg0Y97tjDD2LcVY9ytPSwht8qaM+4orji1uSnRV6lpD0+x?=
 =?us-ascii?Q?IdzsKtui9AsAi96cCL9FBcy0hYguRm340BBb7IEDXsmvhQY8kYzsp1haJZPk?=
 =?us-ascii?Q?Gg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: kr6rRn8icO8jOMc+1fB68wgaGABeKHTA0sGNz8ktuRCA17446ao8+vkm0YbNk5vhANpRHwaVqK3805lnHuTkxSKfBsvh0lHQApiMRkyPaFvAY8aN556BHR1j80eRvaDl13S1+w0k9rmp+/pDw1wEDsesyxzUMyRSGRUOvwIMb6+AnkriLiWrucg65Bb26qTj/wQiJo2BeZZGczJuUyoO2pFlSFk7107GJvc8mYPOFXDJI+Ehg0mRt5e/NiJI28wJ6HIvKcw2gcUqZJ5cmFXE2aBZ0V9QlHdYrf0X9NgD0hbjbxkN+RGXYAb7dkXhV/jWX7UapSQdOmM1a+5x/yTcswyj94Qa3InFlUr/oOgVkOq+/0WVHZ6FqZg+H7EpZFYn1ZEXBpS2PhY7jXFcpDJncLs90RGrHE7mcHK+PF35zTZb9tpdmWZVEx709QJU8ty6xQ73+1PPX4UxROlec31lmrkL5QOLyY3BODjJ1KuigC645xhx+LQWavq0YOjNWxOTgpBxVPmXpBEOhfSPRbR+Enw0/9g8GvLiCsfEq9G1LI/pxnTyNeSm7BpDVnvWGicKdcVeIvL2OsphvO4v7AiIJnUSG1Q410ZVBGnzBj8fKgKCc3fohMkmuLeQg5kPfB6uqlUjRvqb9NS1aeisPu0jtGzkMdhSCswK7+Gq7zn1V85ugv8CnuI+pV1WlBWP4ka9nRe2OZ/CoHNaJygtzCzpM4ek0UvdbdLMsmQrykS7o004T0wK+PXYBfYhXJ+YiuISAWhZWZXCPRiV3u11ESB4Jx1gAoFx8qyvIES22oOqoazOds8YkeUS7Ld7sahDYR0+E/hIuygB0D3x8jGYeZkqeQ==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 32cea3ad-8a26-4660-0357-08dbcc002946
X-MS-Exchange-CrossTenant-AuthSource: SA1PR10MB5711.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2023 15:22:06.9899
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: iRizX9fcqvpyUCcF8p/ARRwu/PLIkjbHKyj+DdGPpoT/3yzv9//OvJbj7LBIlaxUCUwQ6fXsIpxd6yvH/hQeaI/Ff9mu1GjBspXtBpo5qx0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB6431
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-13_06,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0
 mlxlogscore=999 adultscore=0 suspectscore=0 phishscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2309180000 definitions=main-2310130129
X-Proofpoint-ORIG-GUID: pfl5vkIFMdGLZfW_lM1aEH-jN3APvVBT
X-Proofpoint-GUID: pfl5vkIFMdGLZfW_lM1aEH-jN3APvVBT
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Fri, 13 Oct 2023 08:22:18 -0700 (PDT)

Hi,

On Sat, Oct 07, 2023 at 11:52:33AM +0800, Wang Jinchao wrote:
> Signed-off-by: Wang Jinchao <wangjinchao@xfusion.com>

The Fixes: tag should be near your Signed-off-by:, and the subject of
the patch should be something descriptive like

    padata: Fix refcnt handling in padata_free_shell()

Here's some documentation about this:

    https://docs.kernel.org/process/5.Posting.html#patch-formatting-and-changelogs

> Acked-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> ---
> V3: 
>     Include Daniel's ack
>
> V2: https://lore.kernel.org/all/ZRTLHY5A+VqIKhA2@fedora/
>     To satisfy Sparse, use rcu_dereference_protected.
>     Reported-by: kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202309270829.xHgTOMKw-lkp@intel.com/

These two tags can also go near your SoB.

> V1: https://lore.kernel.org/all/ZRE4XvOOhz4HSOgR@fedora/
> 
>  kernel/padata.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/padata.c b/kernel/padata.c
> index 222d60195de6..79d04a97ded6 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -1102,12 +1102,16 @@ EXPORT_SYMBOL(padata_alloc_shell);
>   */
>  void padata_free_shell(struct padata_shell *ps)
>  {
> +	struct parallel_data *pd;
> +
>  	if (!ps)
>  		return;
>  
>  	mutex_lock(&ps->pinst->lock);
>  	list_del(&ps->list);
> -	padata_free_pd(rcu_dereference_protected(ps->pd, 1));
> +	pd = rcu_dereference_protected(ps->pd, 1);
> +	if (refcount_dec_and_test(&pd->refcnt))
> +		padata_free_pd(rcu_dereference_protected(ps->pd, 1));

As Herbert points out, this version changes the code by
rcu-dereferencing pd twice.  Usually previous acks are dropped when
introducing new changes, but you can have it back by only
rcu-dereferencing once and addressing the other comments above.

>  	mutex_unlock(&ps->pinst->lock);
>  
>  	kfree(ps);
> -- 
> 2.40.0
>