From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753882AbcHPPWK (ORCPT ); Tue, 16 Aug 2016 11:22:10 -0400 Received: from mail-sn1nam02on0115.outbound.protection.outlook.com ([104.47.36.115]:8471 "EHLO NAM02-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753466AbcHPPWH (ORCPT ); Tue, 16 Aug 2016 11:22:07 -0400 From: "Kani, Toshimitsu" To: "dan.j.williams@intel.com" CC: "linux-kernel@vger.kernel.org" , "linux-nvdimm@lists.01.org" Subject: Re: [PATCH] libnvdimm: Fix nvdimm_probe error on NVDIMM-N Thread-Topic: [PATCH] libnvdimm: Fix nvdimm_probe error on NVDIMM-N Thread-Index: AQHR9x3imVa4UxOQE0euugE9aPOgTKBKh5MAgAEpcAA= Date: Tue, 16 Aug 2016 15:05:48 +0000 Message-ID: <1471359928.32015.57.camel@hpe.com> References: <1471283545-16561-1-git-send-email-toshi.kani@hpe.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=toshi.kani@hpe.com; x-originating-ip: [15.219.163.2] x-ms-office365-filtering-correlation-id: 5bcc306e-db73-41c5-674d-08d3c5e6ce7a x-microsoft-exchange-diagnostics: 1;CS1PR84MB0008;6:x7Noc8VP4YOrCXsicm7YyRYDYFqv+lx9uBZqcqpAxHs1MZ8FBrPrxuWd0LEm2CC1KrhY8ziZ5vKkeG/3esNzoNoUGQ6VQX1kTBxVXMLIjh7jn76isUs6L9mqMJAuwIjoWs4z6HmFxGoxYRhJj1+N3uqCyAe1qqOiWx5bZN8EJ5xtVd/ixIcYQobFzHahrp9AQDTm9KtAdnAC1wwtWWaC6cTdysjV4DPLJQ3KGQoJn6VsZuM3OJM/HHWEUyAB/9IZ8cDON9ACkGSg6L2r7pse2pMiv3CanjABLYHP2p7oG9M4RQWL1g/I1qlvtmFQoKq8pJmz56+zw88j0iVOOFu7ww==;5:bVMrkAzJVmeZQTvUgw+g2akPp0beZ0g4ARtE1bfCQpY85Fe+4RT9m5zyp2Lgv5o1hczl+4AnN1F9/8AXXoUKvCTLRYGeIjfziEV75ag/gFO+0bv4tKqJcZVl3cMAizISMfEP7f/8bgUlQ/IIhkRb1g==;24:9ntUDgnBflDabcncyNq0Lqs0QmVRl024+TKi7Ddr7qPVDG3gxT7zwh67ftws4gwQLL53awQ9OeQFkamZS96tieywD7sR1wEgywfoKe53RfI=;7:f9G7cp/EbBpQpb02ySoD7yXWg3EHKdDyn4wmgAhiGpgE3WYMJ/W/4Fx/QTZBt7DWfaYJCwKKy50Qe3e3/df5UOxfH6lrRU4n2Y0BAVAgggiooDXPfhAja89ftlB+fRCCDCtju2P9DZoDayosvrWgCD5EmGTR2UMTDnNCsBbI5TCouMDqI/v/GXVv80080W2UN5M0RCInNm3InAhs7aUy5vsAKP8kh2muU0YdOE3BMoPNbMMP0c/AUtBCyTP+klVG x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0008; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(227479698468861)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:CS1PR84MB0008;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0008; x-forefront-prvs: 0036736630 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(7916002)(377424004)(24454002)(189002)(199003)(377454003)(2501003)(8676002)(102836003)(3846002)(87936001)(66066001)(3280700002)(11100500001)(2950100001)(36756003)(6116002)(19580405001)(586003)(33646002)(2900100001)(5890100001)(2906002)(92566002)(4326007)(97736004)(305945005)(77096005)(8936002)(54356999)(122556002)(19580395003)(7736002)(2351001)(7846002)(10400500002)(76176999)(50986999)(99286002)(86362001)(81156014)(106356001)(5640700001)(101416001)(68736007)(3660700001)(5002640100001)(106116001)(103116003)(105586002)(189998001)(110136002)(81166006);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR84MB0008;H:CS1PR84MB0005.NAMPRD84.PROD.OUTLOOK.COM;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" Content-ID: <927FE5ABFB4F44498EF2A8CFF81D932D@NAMPRD84.PROD.OUTLOOK.COM> MIME-Version: 1.0 X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Aug 2016 15:05:48.4207 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0008 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u7GFMFNG010640 On Mon, 2016-08-15 at 14:20 -0700, Dan Williams wrote: > On Mon, Aug 15, 2016 at 10:52 AM, Toshi Kani > wrote: > > > > 'ndctl list --buses --dimms' does not list any NVDIMM-Ns since > > they are considered as idle.  ndctl checks if any driver is > > attached to nmem device.  nvdimm_probe() always fails in > > nvdimm_init_nsarea() since NVDIMM-Ns do not implement optinal > > ND_CMD_GET_CONFIG_DATA command. > > > > Change nvdimm_probe() to accept the case that the CONFIG_DATA > > command is not implemented for NVDIMM-Ns.  The driver attaches > > without ndd, which keeps it no-op to the device. > > > > Reported-by: Brian Boylston > > Signed-off-by: Toshi Kani > > Cc: Dan Williams > > --- > >  drivers/nvdimm/dimm.c      |   10 ++++++++++ > >  drivers/nvdimm/dimm_devs.c |   27 ++++++++++++++++----------- > >  drivers/nvdimm/nd.h        |    1 + > >  3 files changed, 27 insertions(+), 11 deletions(-) > > This fails the ndctl unit test suite, see below plus some other > cleanups... Sorry, I had overlooked about this test suite... I built it to run the test and was able to reproduce the failures. > > > > diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c > > index 71d12bb..07e09c2 100644 > > --- a/drivers/nvdimm/dimm.c > > +++ b/drivers/nvdimm/dimm.c > > @@ -26,6 +26,13 @@ static int nvdimm_probe(struct device *dev) > >         struct nvdimm_drvdata *ndd; > >         int rc; > > > > +       rc = nvdimm_check_config_data(dev); > > +       if (rc == -ENOENT) > > +               /* not required for non-aliased nvdimm, ex. NVDIMM- > > N */ > > +               return 0; > > +       else > > +               return rc; > > + > > Change usage of ENOENT to ENOTTY throughout... Will do. > > > >         ndd = kzalloc(sizeof(*ndd), GFP_KERNEL); > >         if (!ndd) > >                 return -ENOMEM; > > @@ -72,6 +79,9 @@ static int nvdimm_remove(struct device *dev) > >  { > >         struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); > > > > +       if (!ndd) > > +               return 0; > > + > >         nvdimm_bus_lock(dev); > >         dev_set_drvdata(dev, NULL); > >         nvdimm_bus_unlock(dev); > > diff --git a/drivers/nvdimm/dimm_devs.c > > b/drivers/nvdimm/dimm_devs.c > > index d9bba5e..fee82d3 100644 > > --- a/drivers/nvdimm/dimm_devs.c > > +++ b/drivers/nvdimm/dimm_devs.c > > @@ -28,28 +28,33 @@ static DEFINE_IDA(dimm_ida); > >   * Retrieve bus and dimm handle and return if this bus supports > >   * get_config_data commands > >   */ > > -static int __validate_dimm(struct nvdimm_drvdata *ndd) > > +int nvdimm_check_config_data(struct device *dev) > >  { > > -       struct nvdimm *nvdimm; > > - > > -       if (!ndd) > > -               return -EINVAL; > > - > > -       nvdimm = to_nvdimm(ndd->dev); > > +       struct nvdimm *nvdimm = to_nvdimm(dev); > > > >         if (!nvdimm->cmd_mask) > > -               return -ENXIO; > > +               goto err; > >         if (!test_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm->cmd_mask)) > > -               return -ENXIO; > > +               goto err; > > > >         return 0; > > + > > + err: > > +       if (nvdimm->flags & NDD_ALIASING) > > +               return -ENXIO; > > +       else > > +               return -ENOENT; > > > Let's not use "goto" since there is nothing to unwind. Got it. > > > >  } > > > >  static int validate_dimm(struct nvdimm_drvdata *ndd) > >  { > > -       int rc = __validate_dimm(ndd); > > +       int rc; > > > > -       if (rc && ndd) > > +       if (!ndd) > > +               return -EINVAL; > > Since we've called nvdimm_check_config_data() before allocating ndd > it will always be NULL causing DIMMs with label areas to fail init. I am still puzzled at the moment, but will look into the issue. Thanks! -Toshi