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:     Jens Axboe <axboe@fb.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Marcin Dziegielewski <marcin.dziegielewski@intel.com>,
        "Konopko, Igor J" <igor.j.konopko@intel.com>
Subject: Re: [GIT PULL 18/20] lightnvm: pblk: handle case when mw_cunits
 equals to 0
Thread-Topic: [GIT PULL 18/20] lightnvm: pblk: handle case when mw_cunits
 equals to 0
Thread-Index: AQHT9nNmZ5d7NBWAqUW0dpHncmqcfg==
Date:   Mon, 28 May 2018 11:02:43 +0000
Message-ID: <54B2CCDB-B869-4087-8AE2-2AC73381B1FF@cnexlabs.com>
References: <20180528085841.26684-1-mb@lightnvm.io>
 <20180528085841.26684-19-mb@lightnvm.io>
In-Reply-To: <20180528085841.26684-19-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=_546A7123-79FB-4016-A5C3-2B192E5D9485";
        protocol="application/pgp-signature"; micalg=pgp-sha512
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 607d2810-ea29-4b20-6203-08d5c48a8953
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 May 2018 11:02:43.0173
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: e40dfc2e-c6c1-463a-a598-38602b2c3cff
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO2PR06MB491
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=_546A7123-79FB-4016-A5C3-2B192E5D9485
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

> On 28 May 2018, at 10.58, Matias Bj=C3=B8rling <mb@lightnvm.io> wrote:
>=20
> From: Marcin Dziegielewski <marcin.dziegielewski@intel.com>
>=20
> Some devices can expose mw_cunits equal to 0, it can cause creation
> of too small write buffer and cause performance to drop on write
> workloads.
>=20
> To handle that, we use the default value for MLC and beacause it
> covers both 1.2 and 2.0 OC specification, setting up mw_cunits
> in nvme_nvm_setup_12 function isn't longer necessary.
>=20
> Signed-off-by: Marcin Dziegielewski <marcin.dziegielewski@intel.com>
> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
> Signed-off-by: Matias Bj=C3=B8rling <mb@lightnvm.io>
> ---
> drivers/lightnvm/pblk-init.c | 10 +++++++++-
> drivers/nvme/host/lightnvm.c |  1 -
> 2 files changed, 9 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/lightnvm/pblk-init.c =
b/drivers/lightnvm/pblk-init.c
> index d65d2f972ccf..0f277744266b 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -356,7 +356,15 @@ static int pblk_core_init(struct pblk *pblk)
> 	atomic64_set(&pblk->nr_flush, 0);
> 	pblk->nr_flush_rst =3D 0;
>=20
> -	pblk->pgs_in_buffer =3D geo->mw_cunits * geo->all_luns;
> +	if (geo->mw_cunits) {
> +		pblk->pgs_in_buffer =3D geo->mw_cunits * geo->all_luns;
> +	} else {
> +		pblk->pgs_in_buffer =3D (geo->ws_opt << 3) * =
geo->all_luns;
> +		/*
> +		 * Some devices can expose mw_cunits equal to 0, so =
let's use
> +		 * here default safe value for MLC.
> +		 */
> +	}
>=20
> 	pblk->min_write_pgs =3D geo->ws_opt * (geo->csecs / PAGE_SIZE);
> 	max_write_ppas =3D pblk->min_write_pgs * geo->all_luns;
> diff --git a/drivers/nvme/host/lightnvm.c =
b/drivers/nvme/host/lightnvm.c
> index 41279da799ed..c747792da915 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -338,7 +338,6 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 =
*id,
>=20
> 	geo->ws_min =3D sec_per_pg;
> 	geo->ws_opt =3D sec_per_pg;
> -	geo->mw_cunits =3D geo->ws_opt << 3;	/* default to MLC safe =
values */
>=20
> 	/* Do not impose values for maximum number of open blocks as it =
is
> 	 * unspecified in 1.2. Users of 1.2 must be aware of this and =
eventually
> --
> 2.11.0

By doing this, 1.2 future users (beyond pblk), will fail to have a valid
mw_cunits value. It's ok to deal with the 0 case in pblk, but I believe
that we should have the default value for 1.2 either way.

A more generic way of doing this would be to have a default value for
2.0 too, in case mw_cunits is reported as 0.

Javier


--Apple-Mail=_546A7123-79FB-4016-A5C3-2B192E5D9485
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+qZPG1bJoyIX4xUKFRnnQFAlsL4dAACgkQIX4xUKFR
nnSKshAAkdcTIrvO8vsPZqwryh0tsEh4CawV7n4kB8wxTWEZEqTiusIObxyHsx+U
0ZeZI+FLmI4gxb9J7fNzTs7tge+gXUEUQpeRi9U+6cfE7HXC8Tlc/taqT8npxebx
bpeOvr5zLPlo2yRTSCa1OveZoDhz+uG/fd/LAOXG24r+2pR+dlZvkQZBptHywyiQ
Q/jWdm/qhuXldRniz76ahiUYbtblrolx3K94y0CDVTnM1li33xmjlBw6jPckg2+7
Dyt/t43/Rldoce3WBsXGOSiA6GoDixnaMyhPijcAZwT1RxqvuzTGc6bAB3otep/3
Q/tF7DX/yJUTp6paRZF0yMzeat6r+mDcAMfIYqsjTOGS2Avdd79OJDhQtTPn9j8o
8jzrvKAivdEy/LsZyOxjIZF+pXMQ50Dt2CAIhPIlX4Oz8e+YZOifv74jQTh56CsL
80P0tFrBu22J1wrMYCJlyzdtYvvBSn1obcnpZ/C7w/1j8crge3uBUBm2ZrEx6jtO
h92bAoUqB+ojdnTPDYaK7AiL/qmL1RvXgWLiNewVp1DiyCT1TECKguf2SI0GKFJR
upilYJnkEYnG0Owr0mTyKq5XYGdv4tn8KbCICTCQkHapXGAkrnTq+/322vFEhMzM
mmKcmroMlVZWIvC/d9YSYZswA/nDmDaSix3L7l5IMU/HbwQnRoU=
=kcEi
-----END PGP SIGNATURE-----

--Apple-Mail=_546A7123-79FB-4016-A5C3-2B192E5D9485--