Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Javier Gonzalez <javier@cnexlabs.com>
To:     =?utf-8?B?TWF0aWFzIEJqw7hybGluZw==?= <mb@lightnvm.io>
CC:     "Konopko, Igor J" <igor.j.konopko@intel.com>,
        "marcin.dziegielewski@intel.com" <marcin.dziegielewski@intel.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Keith Busch <keith.busch@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH] lightnvm: simplify geometry enumeration
Thread-Topic: [PATCH] lightnvm: simplify geometry enumeration
Thread-Index: AQHUh+/AxF+u6YXd40KvE3EuBX7KZaVn6b6A
Date:   Fri, 30 Nov 2018 07:17:54 +0000
Message-ID: <6FEED0D7-5064-464A-B522-412CEE2A1C7A@cnexlabs.com>
References: <20181129142738.9567-1-mb@lightnvm.io>
In-Reply-To: <20181129142738.9567-1-mb@lightnvm.io>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: cnexlabs.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/signed;
        boundary="Apple-Mail=_8CC13221-B825-4BDC-BFC5-4075621773DC";
        protocol="application/pgp-signature";
        micalg=pgp-sha512
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 15d8eb01-b169-4f60-2c63-08d65693f285
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Nov 2018 07:17:54.7558
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: e40dfc2e-c6c1-463a-a598-38602b2c3cff
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR06MB5210
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

--Apple-Mail=_8CC13221-B825-4BDC-BFC5-4075621773DC
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

> On 29 Nov 2018, at 15.27, Matias Bj=C3=B8rling <mb@lightnvm.io> wrote:
>=20
> Currently the geometry of an OCSSD is enumerated using a two step
> approach:
>=20
> First, nvm_register is called, the OCSSD identify command is issued,
> and second the geometry sos and csecs values are read either from the
> OCSSD identify if it is a 1.2 drive, or from the NVMe namespace data
> structure if it is a 2.0 device.
>=20
> This patch recombines it into a single step, such that nvm_register =
can
> use the csecs and sos fields independent of which version is used. =
This
> enables one to dynamically size the lightnvm subsystem dma pool.
>=20
> Signed-off-by: Matias Bj=C3=B8rling <mb@lightnvm.io>
> ---
> drivers/lightnvm/core.c      | 12 +++++-------
> drivers/nvme/host/core.c     | 18 +++++++++---------
> drivers/nvme/host/lightnvm.c | 18 ++++++------------
> drivers/nvme/host/nvme.h     |  2 --
> 4 files changed, 20 insertions(+), 30 deletions(-)
>=20
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 73ab3cf26868..a1d52e934ce4 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -1145,25 +1145,23 @@ int nvm_register(struct nvm_dev *dev)
> 	if (!dev->q || !dev->ops)
> 		return -EINVAL;
>=20
> +	ret =3D nvm_init(dev);
> +	if (ret)
> +		return ret;
> +
> 	dev->dma_pool =3D dev->ops->create_dma_pool(dev, "ppalist");
> 	if (!dev->dma_pool) {
> 		pr_err("nvm: could not create dma pool\n");
> +		nvm_free(dev);
> 		return -ENOMEM;
> 	}
>=20
> -	ret =3D nvm_init(dev);
> -	if (ret)
> -		goto err_init;
> -
> 	/* register device with a supported media manager */
> 	down_write(&nvm_lock);
> 	list_add(&dev->devices, &nvm_devices);
> 	up_write(&nvm_lock);
>=20
> 	return 0;
> -err_init:
> -	dev->ops->destroy_dma_pool(dev->dma_pool);
> -	return ret;
> }
> EXPORT_SYMBOL(nvm_register);
>=20
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 65c42448e904..387833b37e38 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1516,8 +1516,6 @@ static void __nvme_revalidate_disk(struct =
gendisk *disk, struct nvme_id_ns *id)
> 	if (ns->noiob)
> 		nvme_set_chunk_size(ns);
> 	nvme_update_disk_info(disk, ns, id);
> -	if (ns->ndev)
> -		nvme_nvm_update_nvm_info(ns);
> #ifdef CONFIG_NVME_MULTIPATH
> 	if (ns->head->disk)
> 		nvme_update_disk_info(ns->head->disk, ns, id);
> @@ -3086,13 +3084,6 @@ static void nvme_alloc_ns(struct nvme_ctrl =
*ctrl, unsigned nsid)
> 	nvme_setup_streams_ns(ctrl, ns);
> 	nvme_set_disk_name(disk_name, ns, ctrl, &flags);
>=20
> -	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] =3D=3D =
0x1) {
> -		if (nvme_nvm_register(ns, disk_name, node)) {
> -			dev_warn(ctrl->device, "LightNVM init =
failure\n");
> -			goto out_unlink_ns;
> -		}
> -	}
> -
> 	disk =3D alloc_disk_node(0, node);
> 	if (!disk)
> 		goto out_unlink_ns;
> @@ -3106,6 +3097,13 @@ static void nvme_alloc_ns(struct nvme_ctrl =
*ctrl, unsigned nsid)
>=20
> 	__nvme_revalidate_disk(disk, id);
>=20
> +	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] =3D=3D =
0x1) {
> +		if (nvme_nvm_register(ns, disk_name, node)) {
> +			dev_warn(ctrl->device, "LightNVM init =
failure\n");
> +			goto out_put_disk;
> +		}
> +	}
> +
> 	down_write(&ctrl->namespaces_rwsem);
> 	list_add_tail(&ns->list, &ctrl->namespaces);
> 	up_write(&ctrl->namespaces_rwsem);
> @@ -3119,6 +3117,8 @@ static void nvme_alloc_ns(struct nvme_ctrl =
*ctrl, unsigned nsid)
> 	kfree(id);
>=20
> 	return;
> + out_put_disk:
> +	put_disk(ns->disk);
>  out_unlink_ns:
> 	mutex_lock(&ctrl->subsys->lock);
> 	list_del_rcu(&ns->siblings);
> diff --git a/drivers/nvme/host/lightnvm.c =
b/drivers/nvme/host/lightnvm.c
> index d64805dc8efb..51d957ccf328 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -973,22 +973,11 @@ int nvme_nvm_ioctl(struct nvme_ns *ns, unsigned =
int cmd, unsigned long arg)
> 	}
> }
>=20
> -void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
> -{
> -	struct nvm_dev *ndev =3D ns->ndev;
> -	struct nvm_geo *geo =3D &ndev->geo;
> -
> -	if (geo->version =3D=3D NVM_OCSSD_SPEC_12)
> -		return;
> -
> -	geo->csecs =3D 1 << ns->lba_shift;
> -	geo->sos =3D ns->ms;
> -}
> -
> int nvme_nvm_register(struct nvme_ns *ns, char *disk_name, int node)
> {
> 	struct request_queue *q =3D ns->queue;
> 	struct nvm_dev *dev;
> +	struct nvm_geo *geo;
>=20
> 	_nvme_nvm_check_size();
>=20
> @@ -996,6 +985,11 @@ int nvme_nvm_register(struct nvme_ns *ns, char =
*disk_name, int node)
> 	if (!dev)
> 		return -ENOMEM;
>=20
> +	/* Note that csecs and sos will be overridden if it is a 1.2 =
drive. */
> +	geo =3D &dev->geo;
> +	geo->csecs =3D 1 << ns->lba_shift;
> +	geo->sos =3D ns->ms;
> +
> 	dev->q =3D q;
> 	memcpy(dev->name, disk_name, DISK_NAME_LEN);
> 	dev->ops =3D &nvme_nvm_dev_ops;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 9fefba039d1e..4f5399c8808b 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -541,13 +541,11 @@ static inline void nvme_mpath_stop(struct =
nvme_ctrl *ctrl)
> #endif /* CONFIG_NVME_MULTIPATH */
>=20
> #ifdef CONFIG_NVM
> -void nvme_nvm_update_nvm_info(struct nvme_ns *ns);
> int nvme_nvm_register(struct nvme_ns *ns, char *disk_name, int node);
> void nvme_nvm_unregister(struct nvme_ns *ns);
> extern const struct attribute_group nvme_nvm_attr_group;
> int nvme_nvm_ioctl(struct nvme_ns *ns, unsigned int cmd, unsigned long =
arg);
> #else
> -static inline void nvme_nvm_update_nvm_info(struct nvme_ns *ns) {};
> static inline int nvme_nvm_register(struct nvme_ns *ns, char =
*disk_name,
> 				    int node)
> {
> --
> 2.17.1
>=20
>=20
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

Good refactor; much better to do initialization on a single step.

Reviewed-by: Javier Gonz=C3=A1lez <javier@cnexlabs.com>


--Apple-Mail=_8CC13221-B825-4BDC-BFC5-4075621773DC
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE+ws7Qq+qZPG1bJoyIX4xUKFRnnQFAlwA5CEACgkQIX4xUKFR
nnTNAg/+If42ZquryLgdIeq5uXcr6icmtLkPKGLYbimbcKyue+D6kEVC88gGwk4i
kd60Q6afKyYFNYHay83QlryFjepTJqh/LrVxhnuVHODLYfe89Ei1zm0HtsBXyEym
IYqS26+KiYyv9LE+eJbWoicPPKKgYR2IWF3i0fAm2IzebftHp4G+wLiWOVrswq6r
aMNugBp2YI+lCyh9qNtnFT2u81pgkjIzcXe08v705+rDNJBvPNp0sYT8un+3/rqP
o8ztY9v9DUOVbff1reFO/Xg0FV0PjtPqUSbrfDIo39sZXWGbXkDs4YnafkM902d1
IXWeHGGRzCZzh7vfRb4glksTNh0aa22eyVU1n9Am8gWXpC5/d9d/xrUje45E1sDA
jH4MnmQ41c/467pbIbLMnwPqJ6+IgIsL9/mcupmORyeFaH+qyWG8drmwTaz7ZATU
TDsbcuErlIHt/oK5ffJ82doUAS1BPX+/8V9P5EHdkruxZx+zakmJEUC26Mnn63Ba
9uqRc+kE9Lc6dqTTYhRzAJkGfedrJxdzsBK/gepeQjxUSmv+IMnm1R+B3d/9NDAu
pwDJyZzN7LssALqfXmvaICeUYG5hXKKKdoXRhR/pFEnRx9qNvTEBpczXJ60X5YHZ
Hk8c17N1Jrwqz6fg5/4tb/gF0ELbuadAXrb5r2EgXGkMUP3NFmQ=
=tbRG
-----END PGP SIGNATURE-----

--Apple-Mail=_8CC13221-B825-4BDC-BFC5-4075621773DC--