From patchwork Wed Jan 31 16:04:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Ledford X-Patchwork-Id: 10194173 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B0080603EE for ; Wed, 31 Jan 2018 16:04:35 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D63BE28717 for ; Wed, 31 Jan 2018 16:04:35 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CA7362871B; Wed, 31 Jan 2018 16:04:35 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2AA2E2871A for ; Wed, 31 Jan 2018 16:04:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932257AbeAaQEe (ORCPT ); Wed, 31 Jan 2018 11:04:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43818 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932166AbeAaQEd (ORCPT ); Wed, 31 Jan 2018 11:04:33 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1C0542D0FB1; Wed, 31 Jan 2018 16:04:33 +0000 (UTC) Received: from haswell-e.nc.xsintricity.com (ovpn-120-73.rdu2.redhat.com [10.10.120.73]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5E29F60BE5; Wed, 31 Jan 2018 16:04:32 +0000 (UTC) Message-ID: <1517414670.19117.16.camel@redhat.com> Subject: Re: [bug report] RDMA/bnxt_re: Add SRQ support for Broadcom adapters From: Doug Ledford To: Leon Romanovsky , Dan Carpenter Cc: Devesh Sharma , linux-rdma Date: Wed, 31 Jan 2018 11:04:30 -0500 In-Reply-To: <20180131064829.GR2055@mtr-leonro.local> References: <20180130124546.GA9394@mwanda> <20180131063238.ch4mcl7c6ps7ykxe@mwanda> <20180131064829.GR2055@mtr-leonro.local> Organization: Red Hat, Inc. Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 31 Jan 2018 16:04:33 +0000 (UTC) Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2018-01-31 at 08:48 +0200, Leon Romanovsky wrote: > On Wed, Jan 31, 2018 at 09:32:38AM +0300, Dan Carpenter wrote: > > On Wed, Jan 31, 2018 at 11:16:55AM +0530, Devesh Sharma wrote: > > > On Tue, Jan 30, 2018 at 6:15 PM, Dan Carpenter wrote: > > > > Hello Devesh Sharma, > > > > > > > > The patch 37cb11acf1f7: "RDMA/bnxt_re: Add SRQ support for Broadcom > > > > adapters" from Jan 11, 2018, leads to the following static checker > > > > warning: > > > > > > > > drivers/infiniband/hw/bnxt_re/ib_verbs.c:1317 bnxt_re_destroy_srq() > > > > warn: 'srq->umem' isn't an ERR_PTR > > > > > > > > drivers/infiniband/hw/bnxt_re/ib_verbs.c > > > > 1313 dev_err(rdev_to_dev(rdev), "Destroy HW SRQ failed!"); > > > > 1314 return rc; > > > > 1315 } > > > > 1316 > > > > 1317 if (srq->umem && !IS_ERR(srq->umem)) > > > > ^^^^^^^^^^^^^^^^ > > > > We never store error pointers to srq->umem. It's pretty consistently > > > > checked for error pointers though so maybe that's fine. It causes a > > > > static checker warning because error pointer confusion is a pretty > > > > common source of bugs. Anyway, feel free to ignore if you want... > > > > > > Thanks for reporting Dan, > > > > > > Is there a way out, I want to call ib_umem_release only if it was valid. > > > I think if ib_umem_release checks for the validity of pointer then I > > > can get rid of this? > > > There are other places also in bnxt_re driver where such checks are present. > > > > Yeah. Those places generate warnings as well, but I thought one was > > enough. It's fine if you want to ignore the warning, no one will be > > upset. :P > > Not really, we are trying to clean the subsystem from the warnings > and driver authors who ignore such warnings simply and very effective > sabotage it. > > Currently my checks print ~400 warnings for the drivers/infiniband/* + > drivers/net/ethernet/mellanox/* > > So please don't increase this number, or fix the driver or fix the tool :) > > Thanks Looking at the code, the proper fix for this is: [dledford@haswell-e linus (k.o/wip/dl-for-next *)]$ git diff return ERR_PTR(rc); [dledford@haswell-e linus (k.o/wip/dl-for-next *)]$ diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c index 9b8fa77b8831..ae9e9ff54826 100644 --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c @@ -1314,7 +1314,7 @@ int bnxt_re_destroy_srq(struct ib_srq *ib_srq) return rc; } - if (srq->umem && !IS_ERR(srq->umem)) + if (srq->umem) ib_umem_release(srq->umem); kfree(srq); atomic_dec(&rdev->srq_count); @@ -1430,11 +1430,8 @@ struct ib_srq *bnxt_re_create_srq(struct ib_pd *ib_pd, return &srq->ib_srq; fail: - if (udata && srq->umem && !IS_ERR(srq->umem)) { + if (srq->umem) ib_umem_release(srq->umem); - srq->umem = NULL; - } - kfree(srq); exit: