Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <20fc5043-71a0-0401-fbda-ade0a8192c04@oracle.com>
Date:   Sun, 9 Apr 2023 01:16:49 +0530
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.5.1
Subject: Re: [PATCH net] net: wwan: iosm: Fix error handling path in
 ipc_pcie_probe()
Content-Language: en-US
To:     Simon Horman <simon.horman@corigine.com>
Cc:     m.chetan.kumar@intel.com, linuxwwan@intel.com,
        loic.poulain@linaro.org, ryazanov.s.a@gmail.com,
        johannes@sipsolutions.net, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        error27@gmail.com, kernel-janitors@vger.kernel.org,
        vegard.nossum@oracle.com
References: <20230408065607.1633970-1-harshit.m.mogalapalli@oracle.com>
 <ZDGJI8Q6lWCJdEMR@corigine.com>
 <8f47aa3a-9b71-6788-6d75-ccd96dcdb419@oracle.com>
 <ZDHA0uTelrk1BDb7@corigine.com>
From:   Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
In-Reply-To: <ZDHA0uTelrk1BDb7@corigine.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?R2d6aDBocXdaQ21UM3RRbG9uVlFmemZCVlNrMVpQa2hkWVRNcXN4WEJ2UWtm?=
 =?utf-8?B?Z09DSUlYVFRIS0I3M0xpMVNvdS9PY2tPU0J2bXpQd1l2Y1NZOXpWMlgxZHEv?=
 =?utf-8?B?WGpOV2xNY1A4SEh5aG04ZWdCZTNPWkZFeWFvUXNvN21odlc2cFhJVU5wRGxk?=
 =?utf-8?B?VFpmUEVxWXNSSzZpOXpwZmFFSGJJQUJHUHc5NXdpcFNpWlhsSVY4Z0l5d1Vu?=
 =?utf-8?B?VzFRSGVFYUlxbitJVVM3cVd1OHJuK0h0NmN6SFR1SVJ0dElJSE83K3N4QTNj?=
 =?utf-8?B?LzBHQzA1ck8zbFk5SVN5dy9VV1RvRkRQQW1pU1BRV2taOEQ2TWpCc2l4N3ZM?=
 =?utf-8?B?azcyRUlWcEtERXExQnlRZk9pYkk3aGNQRlVOQWxGbjNRYWNST242cEsvb2dx?=
 =?utf-8?B?U1B0VjdSOTNac2h1VVJUNmxuR0duZkxYektpQjBYYVVVLy9xQTZlL29rUTZG?=
 =?utf-8?B?Z1VNVWJvUHFGZFgyWjBJejNlRHBFb3BSK3BnVTVKWHMrQmdQOFlhMURhQUI5?=
 =?utf-8?B?ZUQ1WE5yYzhHTVBFUTYrdGdHTHMySExLOFpXZDMxK294WDFKSjVGUlg2cWhK?=
 =?utf-8?B?SGE0KzRvS1hkOE0weklBUVlzTjR0YmthUm1HYWdhc0djVVpCdjg4N0xxYXFS?=
 =?utf-8?B?M1g0MzJGVTNHd1ZKTXJzL1ZGNTBYZFpDSGpaOTZRb1NUKytxZnUwQmRTdHNZ?=
 =?utf-8?B?QWxCdERkakNyZ0tycWdOVGlhZHBEaTVJUTFBVUkrWWJhWlE1MlR5aHpXNEY3?=
 =?utf-8?B?SGpQbDFiUXFDckZjY3h6TzdXb2xFV2c0VVVFUmRYQ1dMbHNUMTc3YzY3Q0ph?=
 =?utf-8?B?cW56bGxWYWdMamlOeXdUOVFQTmYrRGxEZmIvWnRUZ0U0WUVBRXk2RGJrZThT?=
 =?utf-8?B?VkRETE16cElJeFRLek1sZ3VxRGtOWFJYMDVZVVpPallSS0M3SVB3cGdTdXND?=
 =?utf-8?B?K3hKUUt5ZVlMWHVFblA5cVlYZnlXMjlac3VFb3N2anhoK2ZpcnBydWpKaTFX?=
 =?utf-8?B?VDcwMlVFdkg5TXFJUnFhOWEvZnBKODg2a2hLSS9XVi9UeUxpOVVDSkxtSG9E?=
 =?utf-8?B?YmtUcWZjcWNLSnRMNldvSUhTSG1XL0tBeVAzMFVheklRcSszR3pNVFkxNTF4?=
 =?utf-8?B?WTZ6cWdkUXZ6cERDSm9rc2xCeXlRU1EycC8vRWJoTnFxZkc4Q2VwY0NJb3Qx?=
 =?utf-8?B?UkpNUnZCcllCeUFTRFVlZFU2NDd6TEx2YWtCNklkL096WWRqVE4yU0F4c2gz?=
 =?utf-8?B?djN6Z2hPRG1oV2lXMHI3c1JpaWFTUG1DTTYyOFVpNUZDWmFXL2RBTUEwY1Az?=
 =?utf-8?B?TVdMdDVuc3RoN0x5UE9nbWl6OEVZb3JHTUdrR2lEdE1kM1hTNGIrem5pOUY2?=
 =?utf-8?B?NndFc1QwQU1SQWVzZjNFaUVtWWlsMFJvZDFFYkpJQmVHOTRyV01UcjZSUVVR?=
 =?utf-8?B?RlBaV0pWOUlkbGw4UWxpaWFUM2xCZUt1OGZ3ZEk4VHczWUsrMmNYMWhsN2VS?=
 =?utf-8?B?Z2djeWFNdVVpVVc3aXdMUERlZ2lBTUNOSlZlOXF2aHVUR3pFR3pISVJsY1hM?=
 =?utf-8?B?TVNoeUJFS0RZVmRxdTNuTkoxMEhkYkxIK2V5bkJLckd1NXpNSW9nV1doTUg2?=
 =?utf-8?B?a0F0dFhFbFQ0RjZkR1pBcFVjeE1TaExXY29STHFJNlVmTmNZdHI1NzkxNUxl?=
 =?utf-8?B?YWthaTZSQkF6RDg3eHZJOHN4SmhmcjhPQkMwZVNjdisxRGIwYmpDb3RjM3Jk?=
 =?utf-8?B?Y0JzMEVQRWNoN1BDdTlNQVVMRGJhSHIrV1B6TWhwckQvZmE5R1U5WVdyOU91?=
 =?utf-8?B?NmlxM0UwSDAxWDQxV1hMNXV5THpvL1lSWmVKcFQ2YVdFNW5vNzRjL2Q1WFdj?=
 =?utf-8?B?K1RMdmdCY2xoaGRVbzVHS24yUzVsbmlaNW42OUUvN0NzSURVQjhRTkdsUkJR?=
 =?utf-8?B?THNNUWlGNEhIb2N5TTNqZDVZMVJNOHVaMkZYYkUxVTBac1hiRVR3bFFKR1kw?=
 =?utf-8?B?MUFQcWowT1dzRVJQQTJBSEFnQ3psTFR6REJZSGpRZytIaXdQdGhPRk5CVGhC?=
 =?utf-8?B?djREcEQ2eTUvMk16dDJYOXNaNkFxKzQrTloxQU51UVlkMlduQzdiOThMUTk3?=
 =?utf-8?B?MTNVV0NpV0F6d3JTckRlTHVXZkE3cnN5MG8yUTlPZHpWd0N3akpGeGVycVBJ?=
 =?utf-8?Q?nqYRcY1+JP+nEdNZvwE81YY=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?dDgvSkI4T056Zlp1NUoxSE1aQkJ2WnVUYUc3R1dRR0FJVEw1cmhFR3llYXlU?=
 =?utf-8?B?VlhMcXNTa1QzcDJJY1NoaXFqR3I0WFJuU3g2OVM0SXVra0VRK3p2NWhPdGZz?=
 =?utf-8?B?MUtiWEJUNnlpMnhnVUczS1FkKzkwZ1dTeTNhUjNicis3ZUVxRDdKeXJlQ0hk?=
 =?utf-8?B?akQ1NXJSK2tFM21FQnpGdzQ4dTMrdkVtbVJpL0IxQ3JOaUQzSmFST3c3YWZh?=
 =?utf-8?B?U1FYeWQ2ek8rQ21iYnNhV3loLytlOVNqMzlVVmdkdmp3UkNKVjRoamtYZ0lV?=
 =?utf-8?B?b2xVWjQ4SGpGakZiNVlIYU5Pem1MeDFzNElveTRnekV2U3hOTHNzdkFoME5h?=
 =?utf-8?B?ZE1mTkg2VWVhTUxRL3BHR2lqcmRGQ0dPNUMrbjJkeG5sQ0ZWVXI0aUFyUlhz?=
 =?utf-8?B?YittNXJ5clVlVzNCUzJOMi9lQlF0L1l3YnlBa2NTV1J1RGNhUEc1MktxUk00?=
 =?utf-8?B?TEVaUXlFdXN1R2E5UnJzajd6TVlrd3FDblFQL2pJTXRyTzZLMEdaaG92cUQ2?=
 =?utf-8?B?RmlWZVA1SnNpUGVEOEU0L0pNUU1QQ3RBcTlSaXBlbHJmSHZSbGtZQ2tWdFV5?=
 =?utf-8?B?a0dpYXdyTnF1cFBRY2taSVU3ekVieFdFUG1uREdLbDUxazM2YjI5Vk5VUDkx?=
 =?utf-8?B?d1JhQWlCRnJkVGhhVmlMME9XTmlEbThsTUo5MFZySlNSODJoNGdmSWM4ZWpv?=
 =?utf-8?B?V1RSS2doMVZydGxoR0xlT0E0N1VlVDJFM0g4dy9xTlFOYkJ0NWJwWWxnVFhU?=
 =?utf-8?B?U0FqVkl5SG1mRWFpTXdMK3czMGE0YXZ6ckRYZURrQ2VDZlEwR1NzczlFOG81?=
 =?utf-8?B?d29OYVdDa2ZoZzFrQWFUMnhEL3EzZmd1QWhlc3pDUkQyOGlhcHAzdFhkQ3Rs?=
 =?utf-8?B?Z3JhK1FHUkZuUklzcWFtTkF5UTNsQ2hqcDAweWhkM2d6UHY0K2JCdjQ5cVhs?=
 =?utf-8?B?aVBDcWxSdFR3R0ZBdmYxRVprVXJVWmdFeVFnTWp1WTFXUitWUFlTWVJLbGIy?=
 =?utf-8?B?THZ1QXVoc1FPQ0JpRytoSjJ3T0U4WkI1NzhLWTNFOGNaR2ZSejdSSWpWTW9w?=
 =?utf-8?B?VHRrSzlSNUR1dGltdEx1LzNFa2pVbklNbW5kOENvWGlyL1crbVdCUlBuSlBv?=
 =?utf-8?B?U0R1NWZRNm9WVVEwWjhOSVRLQmhNclcvVERsRlpudUNHQXNnRktiL0UwaWlD?=
 =?utf-8?B?Y1pnOWQ2N2xKRUxDQ1RuczBhYlV5emxQWEZIS3VSTGRTOUZiRkdQTU5tMEQv?=
 =?utf-8?B?U1ZHUXR4Wkk5aE9TZG5OWlVyUlVsNEY3SmpxVm5KNXhFNTVXMXQ5YVRic2tz?=
 =?utf-8?B?VDJNUzNwYjk0RlBidWZjTFRxamtaVkd0K0owdWdTRkt3NGVweG5YaGpRTmtC?=
 =?utf-8?Q?Y7Vjs3buZ1SBA13jZUESewHMJPHKCLP4=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 01d199bd-619a-47c6-46ef-08db386a0664
X-MS-Exchange-CrossTenant-AuthSource: PH8PR10MB6290.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2023 19:47:03.1692
 (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: Q2/jNd7NF9tE4C/vuiyCU5MzIrphN8R4bKagcViYMBQbsfaVYYmO8X25TrGYyzbJdBehke/Mx5tLVOkpOl0FIlvT7seAwRdp/Y0izL5riWm1tm/x4llyhvwdOxrqCIfL
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5056
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-04-08_10,2023-04-06_03,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxscore=0 mlxlogscore=999
 phishscore=0 bulkscore=0 spamscore=0 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2303200000
 definitions=main-2304080178
X-Proofpoint-GUID: -l6CWMRrmOy9e3HEvlj_XpXz3Ry7nYOP
X-Proofpoint-ORIG-GUID: -l6CWMRrmOy9e3HEvlj_XpXz3Ry7nYOP
X-Spam-Status: No, score=-1.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=unavailable
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Simon,

On 09/04/23 1:00 am, Simon Horman wrote:
> On Sat, Apr 08, 2023 at 11:12:25PM +0530, Harshit Mogalapalli wrote:
>> Hi Simon,
>>
>> On 08/04/23 9:02 pm, Simon Horman wrote:
>>> On Fri, Apr 07, 2023 at 11:56:07PM -0700, Harshit Mogalapalli wrote:
>>>> Smatch reports:
>>>> 	drivers/net/wwan/iosm/iosm_ipc_pcie.c:298 ipc_pcie_probe()
>>>> 	warn: missing unwind goto?
>>>>
>>>> When dma_set_mask fails it directly returns without disabling pci
>>>> device and freeing ipc_pcie. Fix this my calling a correct goto label
>>>>
>>>> As dma_set_mask returns either 0 or -EIO, we can use a goto label, as
>>>> it finally returns -EIO.
>>>>
>>>> Renamed the goto label as name of the label before this patch is not
>>>> relevant after this patch.
>>>
>>> nit: I agree that it's nice to name the labels after what they unwind,
>>> rather than where they are called from. But now both schemes
>>> are used in this function.
>>
>> Thanks a lot for the review.
>> I agree that the naming of the label is inconsistent, should we do something
>> like below?
>>
>> diff --git a/drivers/net/wwan/iosm/iosm_ipc_pcie.c
>> b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
>> index 5bf5a93937c9..04517bd3325a 100644
>> --- a/drivers/net/wwan/iosm/iosm_ipc_pcie.c
>> +++ b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
>> @@ -295,7 +295,7 @@ static int ipc_pcie_probe(struct pci_dev *pci,
>>          ret = dma_set_mask(ipc_pcie->dev, DMA_BIT_MASK(64));
>>          if (ret) {
>>                  dev_err(ipc_pcie->dev, "Could not set PCI DMA mask: %d",
>> ret);
>> -               return ret;
>> +               goto set_mask_fail;
>>          }
>>
>>          ipc_pcie_config_aspm(ipc_pcie);
>> @@ -323,6 +323,7 @@ static int ipc_pcie_probe(struct pci_dev *pci,
>>   imem_init_fail:
>>          ipc_pcie_resources_release(ipc_pcie);
>>   resources_req_fail:
>> +set_mask_fail:
>>          pci_disable_device(pci);
>>   pci_enable_fail:
>>          kfree(ipc_pcie);
>>
>>
>>
>> -- but resources_req_fail: has nothing in its block particularly.
> 
> I think this situation is common when one names the labels
> after where they come from. So I'd say this is ok.
> 
Thanks I have a sent a V2 with this.

> An alternative would be to rename all three of labels after what they
> unwind.

Other functions in this file are using similar labels. So may be we 
could with above diff(V2 patch).

Regards,
Harshit