From mboxrd@z Thu Jan 1 00:00:00 1970 From: kbusch@kernel.org (Keith Busch) Date: Fri, 3 May 2019 06:09:15 -0600 Subject: [PATCH v2 08/10] nvme/pci: implement the mdev external queue allocation interface In-Reply-To: References: <20190502114801.23116-1-mlevitsk@redhat.com> <20190502114801.23116-9-mlevitsk@redhat.com> <63a499c3-25be-5c5b-5822-124854945279@intel.com> Message-ID: <20190503120915.GA30013@localhost.localdomain> On Fri, May 03, 2019@12:20:17AM +0300, Maxim Levitsky wrote: > On Thu, 2019-05-02@15:12 -0600, Heitke, Kenneth wrote: > > On 5/2/2019 5:47 AM, Maxim Levitsky wrote: > > > +static void nvme_ext_queue_free(struct nvme_ctrl *ctrl, u16 qid) > > > +{ > > > + struct nvme_dev *dev = to_nvme_dev(ctrl); > > > + struct nvme_queue *nvmeq; > > > + > > > + mutex_lock(&dev->ext_dev_lock); > > > + nvmeq = &dev->queues[qid]; > > > + > > > + if (WARN_ON(!test_bit(NVMEQ_EXTERNAL, &nvmeq->flags))) > > > + return; > > > > This condition is probably not expected to happen (since its a warning) > > but do you need to unlock the ext_dev_lock before returning? > > This is true, I will fix this. This used to be BUG_ON, but due to checkpatch.pl > complains I turned them all to WARN_ON, and missed this. Gentle reminder to trim your replies to the relevant context. It's much easier to read when we don't need to scroll through hundreds of unnecessary lines. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_PASS,T_DKIMWL_WL_HIGH,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68A20C43219 for ; Fri, 3 May 2019 12:15:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 36EDD2089E for ; Fri, 3 May 2019 12:15:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556885705; bh=7CaRcobSQnr8AgSwtp9Rk5pifRZI5NRB81+BJPOTdB0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=CPyNfDck9Ill1ymPEGIXDsv3ijJ66rtxIaNk09QvY/HpeP0fRmJA0CBNISz4N9Svm e9NOqvY/V5t4ImKqbR/c+HRrAhurgjDsnezzI6U1dRRG7BuS43NEI3sFYh6SOwuSOd Vcz4ckGaNTTDKarNmRkAObiOajkQ85wXTjhFqpl4= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726887AbfECMO7 (ORCPT ); Fri, 3 May 2019 08:14:59 -0400 Received: from mga04.intel.com ([192.55.52.120]:41891 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726047AbfECMO7 (ORCPT ); Fri, 3 May 2019 08:14:59 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 May 2019 05:14:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,425,1549958400"; d="scan'208";a="343043907" Received: from unknown (HELO localhost.localdomain) ([10.232.112.69]) by fmsmga005.fm.intel.com with ESMTP; 03 May 2019 05:14:58 -0700 Date: Fri, 3 May 2019 06:09:15 -0600 From: Keith Busch To: Maxim Levitsky Cc: "Heitke, Kenneth" , linux-nvme@lists.infradead.org, Fam Zheng , Keith Busch , Sagi Grimberg , kvm@vger.kernel.org, "David S . Miller" , Greg Kroah-Hartman , Liang Cunming , Wolfram Sang , linux-kernel@vger.kernel.org, Kirti Wankhede , Jens Axboe , Alex Williamson , John Ferlan , Mauro Carvalho Chehab , Paolo Bonzini , Liu Changpeng , "Paul E . McKenney" , Amnon Ilan , Christoph Hellwig , Nicolas Ferre Subject: Re: [PATCH v2 08/10] nvme/pci: implement the mdev external queue allocation interface Message-ID: <20190503120915.GA30013@localhost.localdomain> References: <20190502114801.23116-1-mlevitsk@redhat.com> <20190502114801.23116-9-mlevitsk@redhat.com> <63a499c3-25be-5c5b-5822-124854945279@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.1 (2017-09-22) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, May 03, 2019 at 12:20:17AM +0300, Maxim Levitsky wrote: > On Thu, 2019-05-02 at 15:12 -0600, Heitke, Kenneth wrote: > > On 5/2/2019 5:47 AM, Maxim Levitsky wrote: > > > +static void nvme_ext_queue_free(struct nvme_ctrl *ctrl, u16 qid) > > > +{ > > > + struct nvme_dev *dev = to_nvme_dev(ctrl); > > > + struct nvme_queue *nvmeq; > > > + > > > + mutex_lock(&dev->ext_dev_lock); > > > + nvmeq = &dev->queues[qid]; > > > + > > > + if (WARN_ON(!test_bit(NVMEQ_EXTERNAL, &nvmeq->flags))) > > > + return; > > > > This condition is probably not expected to happen (since its a warning) > > but do you need to unlock the ext_dev_lock before returning? > > This is true, I will fix this. This used to be BUG_ON, but due to checkpatch.pl > complains I turned them all to WARN_ON, and missed this. Gentle reminder to trim your replies to the relevant context. It's much easier to read when we don't need to scroll through hundreds of unnecessary lines.