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: <8f47aa3a-9b71-6788-6d75-ccd96dcdb419@oracle.com>
Date:   Sat, 8 Apr 2023 23:12:25 +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()
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>
Content-Language: en-US
From:   Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
In-Reply-To: <ZDGJI8Q6lWCJdEMR@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?UjFIcHIvK2hNZ0xSTnBvdTFCWExud2ZJVkhhYzFRa2ErR3lxdVRUTG1iV0tD?=
 =?utf-8?B?QlVTRktsMlE4c0t1bmRkOW9PaGNxVVBSalNrSFcwN1lFeTlsR1dFK0E1WXA5?=
 =?utf-8?B?WGI1SGxKeTR0cC9hVDlvRGVaSlNGRGY1bGJzVm9wTFVmUEZBNmVUN1B5ZzAy?=
 =?utf-8?B?aitCamt3eFBGUCtFek81Z3cxL09TTExEOVFPYk5MUFUzQmJ3ZTQzQmgwbjBx?=
 =?utf-8?B?Yi9Bd0VxSHB0RHg0THV6eGNrZUNmWUwwZFFObFZvWFVnY0JmVGxRSkFkRTV3?=
 =?utf-8?B?ZUtzRWFQZStkRUhQWTdaTkd3UmxnSHJFeHZ2KytiQmVMYS9keTliYW1jbk85?=
 =?utf-8?B?SmJ1S3pKdzQ5Vmlwd1EyekNZbUk4VWNJcHRCaXIxM3FtV0o0c29obWE0VHFO?=
 =?utf-8?B?VGdhdnF5dmF4YTg2Y3I3Y2haMit1ekp2VzJGc1JOeG5SS1R2emJhNHdYUHhi?=
 =?utf-8?B?MU1jdmxBMm9PT3pBQTY5dUl1WGg5TFIySHgxUWtpLzVSZWlzSXRxSUIzQzVq?=
 =?utf-8?B?VGZsalZvTXV5TlRocy9YNi9xOWk3SlY4U29nczd0VHM5K21rVEQ4dDUvVGdx?=
 =?utf-8?B?V3F2M3hOdnQ0RU4vT245Qys1czR1UXhOZjVsTnpkLzRweGhYRThDdFhtSXUx?=
 =?utf-8?B?SExSdGk3YmUwc0p1SWZLMEQ5YndkbVlveDhEdVBuS2RscDdxZ1N1MEUzaE82?=
 =?utf-8?B?aVMyQldLc2kvdGp3U2ZVN25zRjI4TmI4WW9icmZGM0N6dWFvSjhpeXluK2tY?=
 =?utf-8?B?aGJYTy8zeExMOTRvdW9QSmw3R0FaSVlaaHFDVDVDSS85U2xTVVJTYzFJQ2d0?=
 =?utf-8?B?aitic3l5cEh2SUF4NHRSK2FjdXJkY1ZYYjdLelIvM0NkOFB1akpkYk9VT3g5?=
 =?utf-8?B?eFhuMGEvRWpIaEdCZXFwa1N0MzJyd2xpblhsVW95L1d0VFBEdEFoS3JxdWxX?=
 =?utf-8?B?QWg5Z1h1Tk8zSnBGckM0V01NWlRDcS9uZmNncGZ4Sk9vQU1NaDR0b1kxN0Rq?=
 =?utf-8?B?THV2UGNvRmp6TWpsd2RNMVZzU1p2WHZNQnBjd0tzY001NVFPL2tNMGoyb01I?=
 =?utf-8?B?TzlvYkVJa1FqRWRxZ0w1Z3o4Z09tMnMzU2IzUzNCSmdYVUhncy9FUHdtNjRH?=
 =?utf-8?B?VkpnbGhjdDkycytHSy9HSTZsR3BLcm56S3RKWENSRVRwR3RKcTU2bjRxWFFU?=
 =?utf-8?B?Q3VKS09oNHBrQ0VuU1ZEWHJ2dTY5b2wzOXJ6QTRyaWpXSDI4Q2xLRnFxeDJk?=
 =?utf-8?B?RnZqUzNwck5QMy9JVC9zRHhIWUQ3T3pMOG9rT0JyUFlsSXdqMnhBK2VpLzBF?=
 =?utf-8?B?R2xYVDJnVDQwZGdzWkNQVW9OV2tzUllEWEJLbHFUeERXTXY4WW5nZG9pWEc4?=
 =?utf-8?B?OU1TbjFKM1laWkxySEgvTVU4aE54V20xL2FWRURNMVprbitteDdBZVFvTmk5?=
 =?utf-8?B?NUNqell4VTF1MStTaG8velJnMnhFT1UzZGtQYWRXSEI3TlpEWi9namRWalJl?=
 =?utf-8?B?MjFaWmIzRmR4UFJvNkVnZ05QS3NSMGVERndOSDFHRjA2eTRGblgwWWgvV3lj?=
 =?utf-8?B?Z1ZSQlduUm85RXBPeE8rcGNHajhucElXaThYVmpqMXJtOW94aiswL0hrM2NB?=
 =?utf-8?B?Q0ZNM0J0SVJodjc1TDdTSXRlZG80UXJWSTFOWmt6NzBVd1hZNmVLYVNFVHhZ?=
 =?utf-8?B?TnJ4YXp1U3ZPV3NISWpNV2l0WDE3OTlUSC9YQjQ4cEpwUVVDZmEvOWVwSHhJ?=
 =?utf-8?B?aWw0dHI3UmQzWEIwK1BNc214N2Q4VHk1aW81cWJLbmRiTDZRUks1bFNEUDNF?=
 =?utf-8?B?RGpSVFdOaTkzZDdueGI5dnNKaHpGNkR0LzZITXIwZW5rYndyMjVtUk1HQTBI?=
 =?utf-8?B?M2pqaldJSnVzVWdjWDBFRmtjZHc5MFdrdm9JUzFpS1ZhYURlR3k1RHNzZFdj?=
 =?utf-8?B?WjA5SjlDdHJoZ29qaE9zd3JkVnZRQ0Q1VnM1ZC9rbkRGNWp1SDZWckVPRVNt?=
 =?utf-8?B?cTVQY3pMWjAxWHBpUEkyVkFNSXN5cDI0Q1o1NkV0VGQ1NlVBRlhzdElpUmNq?=
 =?utf-8?B?V0U5dFEreGROZTUyS2FUVTc1RERkLzNncHFEU3d1OGhJOThQL21MUG5iVjhX?=
 =?utf-8?B?YjlYYzRVckRRK25nNkdFUEJjUXlYSDlwK2JWYlF6OUNGeU41ZGErRm5kZlRE?=
 =?utf-8?Q?f40ZcfNF/jjlKSuGmv1+eCI=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?NUZwQWFtTjNQVGF0WUhNMnlPK1VtTUVpSXVmRUlTRGJmRHRaa3lZcDVuTkdB?=
 =?utf-8?B?K1BwR05id3ZLSTR5M0ZGYldpUWxieGtHNUdKakd0UDl4Szh1djdpdE5tTnAr?=
 =?utf-8?B?aUhmWGNTUWpVMk96d3RxcVZCR0dtM2t4M20zUlZXb3BySFhzeENqTEg2S2sx?=
 =?utf-8?B?RVdpcklxUkE4MjVQalBUcnRMSlVYUHM1cTB3elFlaUVCR3c2U2cxWStNU1Ju?=
 =?utf-8?B?cGhTUUhZdHVZbmUycExQQ1VsVXRtWkdRYmsvbm0vQzY2VVhLNXliWkViUExE?=
 =?utf-8?B?ZGJFakNOSUxnT0JQb1F4eXZyUzltWExzM0EzQzBGWVc2ejRLSGErNUxLSUlJ?=
 =?utf-8?B?UHdybS8vcHNZLzRhM2YrQjJMRDhIZXhJS1pkNDNKR2lreFB3QU5sUUFaZlU5?=
 =?utf-8?B?SklIMzFqOTJZR3gxWHZ1MW5yMXFCQUt5MlRhdGxJa3RteHI2dFJpbG1uTlJj?=
 =?utf-8?B?UUZ2eng1cWlnbnhvdFdVQTN3dnVNZll5V2EzNXF1QVBWbWQ5QmFLbEFFM3kv?=
 =?utf-8?B?ZG03T0Q3TWdrSUxNT3VZWDZET085NENsMkp2SEs5QnVibTJkaUY5UDVkWmNv?=
 =?utf-8?B?NkZ6NVVaamFyaWNNSnl1NVA2eVlWZjFlOFQ0ajV5L3NrOEJkd1dGOEJTbjVm?=
 =?utf-8?B?YU5tRU5kVHd4Ymx1VFkrckU5UndvZjM5MEJCbCsza3Vtb0JwUzZ0RGxCNEFa?=
 =?utf-8?B?Y04xbW01TS9hcG9jTklrdHZjZVVPR3A0SlJabkZGenkrWFN2SEhnVDZneWRZ?=
 =?utf-8?B?MHBYOVZOUUF3NEN1Lzc5NjQ5S2MvdllqRTVOaDlmM1plaFBKdHM2Wlpza0U3?=
 =?utf-8?B?ckloY0dyZm5aMGtkZlVETG5ZY0s0MFVEcnBaR3ZkT3hGanNmWFQ4T1BRYnBZ?=
 =?utf-8?B?NUNTalEyK0V6cWtPNmxXN3JCdUZmbExRY3FYOVhKNnp3azQ2ZXJicm8wNDdJ?=
 =?utf-8?B?WisvU1pFT2J6Qmxab1dkOFI2cktjdEdqZzdNUllyNTdJalJiL281dlpQTjBo?=
 =?utf-8?B?cW56UGMzaWg1dUN2c2pLWmgxRlFSMGxBVHNEN1JmWGNaUjZaOUcxcVp5VHhw?=
 =?utf-8?B?TDNJQUo1LzhaKyt3RC9TNkxlMm5Jd3pZMjl6djUremFpV1d3YXFWaHJyWE9S?=
 =?utf-8?B?Wi91UHFpTURlTmYrdWJSWEkxS2krZzBvT0dyenFIcTJnWCtpWEt1Z2FLY2xL?=
 =?utf-8?B?V2p2VENxcmJQNURKUFVRbkdhamtZR3NHQzcyNnZpZmRtbWtFVTJ2VVpZWjVW?=
 =?utf-8?B?WTBYbUxib3ZuRmdqOHM2VVpZMVVYUGc0bFZXbWExNVkwVEo3Y2ZDOTRnQjJn?=
 =?utf-8?B?OVF1L2RCeGw2YmRvaWpVL3EzbVMrbFlMR040SHZqQStoTTVmbTZmVHJxSjJT?=
 =?utf-8?Q?huW0NwTTW75G9czTsRpUjGeOI9tNV+r8=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 469e0cef-afd0-471f-563e-08db3858a417
X-MS-Exchange-CrossTenant-AuthSource: PH8PR10MB6290.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2023 17:42:36.8915
 (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: snD0cTiQ9s/0ir9qWNSzzbEd0ej7gDxjfO23LNUUdzGLJQyvnqGTLBegIEmL/HxkWwBYL9ChsOvkKehnveSqVZG8UmM6njxriO6S5cTStEMlrDOaDdG+1zQxDyceOUAk
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB5770
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_08,2023-04-06_03,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0
 suspectscore=0 spamscore=0 mlxlogscore=999 bulkscore=0 malwarescore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304080159
X-Proofpoint-ORIG-GUID: SF_o_kFk2kSCPCCSN5W0EsLmYlcZ-L4P
X-Proofpoint-GUID: SF_o_kFk2kSCPCCSN5W0EsLmYlcZ-L4P
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 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.

Thanks,
Harshit

>>
>> Fixes: 035e3befc191 ("net: wwan: iosm: fix driver not working with INTEL_IOMMU disabled")
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>> This is based on static analysis, only compile tested.
> 
> I agree with your analysis.
> 
> Reviewed-by: Simon Horman <simon.horman@corigine.com>