Message-ID: <3740be2d-192f-aeaf-02fe-e309cdb278dc@oracle.com>
Date:   Fri, 7 Jan 2022 17:49:42 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
Subject: Re: [PATCH] vdpa/mlx5: fix error handling in mlx5_vdpa_dev_add()
Content-Language: en-US
To:     trix@redhat.com, mst@redhat.com, jasowang@redhat.com,
        nathan@kernel.org, ndesaulniers@google.com, elic@nvidia.com,
        parav@nvidia.com, xieyongji@bytedance.com
Cc:     virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, llvm@lists.linux.dev
References: <20220107211352.3940570-1-trix@redhat.com>
From:   Si-Wei Liu <si-wei.liu@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20220107211352.3940570-1-trix@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bmhmVTkxbzRUN0liMVRlRCs4NU5ueURYV3NMQ21QbzFZMXlaR2pZdCt6VVd5?=
 =?utf-8?B?TThaVFFWd21IKzN5U0JPQU5mNGJFc0t0dWpYQ1VWZmVRdE1kNWRLcFhDV3Nu?=
 =?utf-8?B?cXZMbTRZUkt3ZXoyZ0k0bnFZeGsySmJhM3g0cDNhazlLYXRaOWdSZUp5NnJo?=
 =?utf-8?B?d0xoamsyTy9uZHk0aDBJTjgyMHlxTGtBUkNRaXd0M2FuSTBHQXgvR2o3TXRM?=
 =?utf-8?B?QUhFWnRmaUFsWXZiK2Z3N3RGUzBTbFNMNTNSM3ZjdFhsaGZoNVFJSU1MR1Ev?=
 =?utf-8?B?aEZQVVJTTFVVdTg1Y1poenJTT0pUcDV2NnFrQnVPRHNiU1l2QzF4MzN3UEFE?=
 =?utf-8?B?Mnplb3hmdDBuT3FCeU5WWVh3a2VocVd5K3dpM3VEcXhTNUJaQjhrSmk4RVov?=
 =?utf-8?B?LzEvMHlEYTdXL0ZPa0pWeE4xRHI0cHd6VkgyWXI0S0x3ckNJMmwweEJrb1Bu?=
 =?utf-8?B?T09nbDl1eHIvRXQwWkkyRk1CV1FrZEprNW9Pcm9xZEc4MTl2NUk2OEw3R253?=
 =?utf-8?B?OUxjTXpKU3hPM21Qb3F3NkMxcWY0dVJQYlVwTnE2dGhURnhNUzJDbHNwTEFH?=
 =?utf-8?B?YzkxSUdNN1BlaVYxMlFneUp5N0xOMlBwQjNPcFBEaTRZRUJxVURNTGVqenBM?=
 =?utf-8?B?cEJBRGxuMFJvc3lqeFRLNjZGeGVmMUw3c3RCYnZqNy9OWnU5TStleDR3TzVw?=
 =?utf-8?B?eTNqQTN2NmVUejRaYStsZDI5WlJJYjBwQk9FUnV3dDk1eG9Bd2JCbnFUQ2ds?=
 =?utf-8?B?TnFZOXJHd0x4QzkzTjdLN3lLOHJibTJnVDNwaXhsSzFEMW5nUFBmbjJMNTZ0?=
 =?utf-8?B?NXJsRWhTaGV4bmVsUXlPbDF6a0daMFhaMnBnbG1HRFpMdkxJelVaejQ1dlZx?=
 =?utf-8?B?Z2ZyL3FvK01BUFRBZlFzdU9CV1JIK2xDTHBLeGtEcUdFT3lONlVaMy9OaSt6?=
 =?utf-8?B?VjRmdDZwVm5sWUFFVDdxbjVKZFlPeWhvNlcvY2NzTVZNOFBEcVdYV0ZFRnRj?=
 =?utf-8?B?dGF5MEx1Z09EeExFNXR5QXZnamVudmdLU0tZS3hZdVNlN1RvV2U3cXJCNVJT?=
 =?utf-8?B?OG9GdmM2Ungxd0N0WkM3Tk5Lc1RPdDR6eUhDdnpiNG5CM0szaWZZdTcrWlRn?=
 =?utf-8?B?UkZmUnhxTi9icjFzOUphQTVGUW5JVENvU2ozVzZZb0dCMlVNYWlBamFNTVV3?=
 =?utf-8?B?ejNiL3ppa0QzaDBSYStOYVQzTjE3dzJwcXVzM3AwalcyVldXaDl2d25oRXM1?=
 =?utf-8?B?NVc4TU1tYStRMlpsSXdCNS95NTRGZDd4NnRTVG9UNlhQd1F3N08wQ0xyU2Jp?=
 =?utf-8?B?emxwV0VIcmJ2ZjRsNUk5Nk5iNzBqU1lHd05OZ2lOS2dVYURHRW4rSFNERUhr?=
 =?utf-8?B?Y3RGSk1US01TZHdVNGxVWlZLZjlYOTk3ZldvUHlId3Y5QU1nTGRnTXVvMlpR?=
 =?utf-8?B?REorMlB3Y1VjMENvaU8vcThvaEErN3Q4eVF3ZUlqZFBXUlUzYXE4MUNVR2Zm?=
 =?utf-8?B?eU5ObDVmRHpwWGtmV2N1ZUpmUkdKUEltdFFXd3RmU0JsZlFVamdZbFQ2dHhN?=
 =?utf-8?B?SDdhVStzL0c3VlBwc3dhd2UzNG1YSWJWQkRWMVJjcURlUDV4YTVnVm54U1N6?=
 =?utf-8?B?a3pPLzRTNFhPa3I5UDN2UENwZldFUE5ENC83VWlBdzRRVVRZQjY1OUJQem5p?=
 =?utf-8?B?RkZkU2pDb2xxT01nam5OWDVEZ2ZBS1huWjJmVFE5T0d2aVpJRHc3TDhZakVP?=
 =?utf-8?B?Qk05OXUxSHdrc0VnUlA1eXowL2Vxc0tld3dwSExPdy9xdTJNeUo5UHRlZEVo?=
 =?utf-8?B?ZFdSaVdJdUxxQnlWOXVLcW9mNUhNVFo4NDZva3AxcnUrcWhsK1dGNFloR1l3?=
 =?utf-8?B?MmhuOUx6eC9JbENiYjVmZkRZRlErRFBhWENDM0tIUzlxeFFLUFQ0WXRFQlI2?=
 =?utf-8?B?Zjc5WmxHNFB4em5sdlZERjFuem5lcjlIVWVRak5IQmNwWHVTaFFKT1J3NmVt?=
 =?utf-8?B?dGxKaU4vZi92clpPS2VweXdHVlZzdUhuMmtOS0pMc3JSRlVMUlkrMkFhZjla?=
 =?utf-8?B?QVBXK1ZhV2pPZ1lpNjZyeVJVZ3R2M29vQlNVSFZTdUFCdysrQ3dpWkdzdHNs?=
 =?utf-8?B?TmJlMm1UWVJPTk1qc2w1Z1JXMG8zOWI0Q2lrVDJsR3ZValh1MGVVTXNxbWNT?=
 =?utf-8?Q?LxHzpI1WdwpglKEdHh4bR5g=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 260235fc-61b2-423b-2f27-08d9d2492726
X-MS-Exchange-CrossTenant-AuthSource: BN8PR10MB3283.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jan 2022 01:49:48.2613
 (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: nrlrILorL6VauSHh4TTY/mwwG1Wpdqd21dxD65tnriOO4QizX0/nUpvvaMl/PYMPrz/paQhp/eiTDHZq2nX4XQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB5094
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10220 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 mlxscore=0
 phishscore=0 malwarescore=0 spamscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2201080008
X-Proofpoint-ORIG-GUID: DHJdobcfejsf_EeP_Fx710edgW_y0TdM
X-Proofpoint-GUID: DHJdobcfejsf_EeP_Fx710edgW_y0TdM
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


The proposed fix looks fine, but I still hope this to revert this series 
if at all possible. The review hadn't been done yet.

On 1/7/2022 1:13 PM, trix@redhat.com wrote:
> From: Tom Rix <trix@redhat.com>
>
> Clang build fails with
> mlx5_vnet.c:2574:6: error: variable 'mvdev' is used uninitialized whenever
>    'if' condition is true
>          if (!ndev->vqs || !ndev->event_cbs) {
>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mlx5_vnet.c:2660:14: note: uninitialized use occurs here
>          put_device(&mvdev->vdev.dev);
>                      ^~~~~
> This because mvdev is set after trying to allocate ndev->vqs,event_cbs.
> So move the allocation to after mvdev is set but before the arrays
> are used in init_mvqs()
>
> Fixes: 7620d51af29a ("vdpa/mlx5: Support configuring max data virtqueue")
> Signed-off-by: Tom Rix <trix@redhat.com>

Reviewed-by: Si-Wei Liu<si-wei.liu@oracle.com>
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index b564c70475815..37220f6db7ad7 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2569,16 +2569,18 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>   	if (IS_ERR(ndev))
>   		return PTR_ERR(ndev);
>   
> +	ndev->mvdev.mlx_features = mgtdev->mgtdev.supported_features;
> +	ndev->mvdev.max_vqs = max_vqs;
> +	mvdev = &ndev->mvdev;
> +	mvdev->mdev = mdev;
> +
>   	ndev->vqs = kcalloc(max_vqs, sizeof(*ndev->vqs), GFP_KERNEL);
>   	ndev->event_cbs = kcalloc(max_vqs + 1, sizeof(*ndev->event_cbs), GFP_KERNEL);
>   	if (!ndev->vqs || !ndev->event_cbs) {
>   		err = -ENOMEM;
>   		goto err_alloc;
>   	}
> -	ndev->mvdev.mlx_features = mgtdev->mgtdev.supported_features;
> -	ndev->mvdev.max_vqs = max_vqs;
> -	mvdev = &ndev->mvdev;
> -	mvdev->mdev = mdev;
> +
>   	init_mvqs(ndev);
>   	mutex_init(&ndev->reslock);
>   	config = &ndev->config;