On Mon, Jun 18, 2018 at 4:29 PM, Javier Gonzalez [off-list ref] wrot=
e:

quoted

On 16 Jun 2018, at 21.38, Matias Bj=C3=B8rling [off-list ref] wrote:

On 06/16/2018 12:27 AM, Igor Konopko wrote:

quoted

Currently pblk and lightnvm does only check for size
of OOB metadata and does not care wheather this meta
is located in separate buffer or is interleaved with
data in single buffer.
In reality only the first scenario is supported, where
second mode will break pblk functionality during any
IO operation.
The goal of this patch is to block creation of pblk
devices in case of interleaved metadata
Signed-off-by: Igor Konopko <redacted>
---
 drivers/lightnvm/pblk-init.c | 6 ++++++
 drivers/nvme/host/lightnvm.c | 1 +
 include/linux/lightnvm.h     | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.=

c

quoted

quoted

index 5eb641da46ed..483a6d479e7d 100644

--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c

@@ -1238,6 +1238,12 @@ static void *pblk_init(struct nvm_tgt_dev *dev, =

struct gendisk *tdisk,

quoted

quoted

             return ERR_PTR(-EINVAL);
     }
 +   if (geo->ext) {
+            pr_err("pblk: extended (interleaved) metadata in data buff=

er"

quoted

quoted

+                    " not supported\n");
+            return ERR_PTR(-EINVAL);
+    }
+
     pblk =3D kzalloc(sizeof(struct pblk), GFP_KERNEL);
     if (!pblk)
             return ERR_PTR(-ENOMEM);

diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.=

c

quoted

quoted

index 670478abc754..872ab854ccf5 100644

--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c

@@ -979,6 +979,7 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
     geo->csecs =3D 1 << ns->lba_shift;
     geo->sos =3D ns->ms;
+    geo->ext =3D ns->ext;
 }
   int nvme_nvm_register(struct nvme_ns *ns, char *disk_name, int node)

diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 72a55d71917e..b13e64e2112f 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h

@@ -350,6 +350,7 @@ struct nvm_geo {
     u32     clba;           /* sectors per chunk */
     u16     csecs;          /* sector size */
     u16     sos;            /* out-of-band area size */
+    u16     ext;            /* metadata in extended data buffer */
     /* device write constrains */
     u32     ws_min;         /* minimum write size */

I think bool type would be better here. Can it be placesd a bit down, ju=

st over the 1.2 stuff?

quoted

Also, feel free to fix up the checkpatch stuff in patch 1 & 3 & 5.

Apart from Matias' comments, it looks good to me.

Traditionally, we have separated subsystem and target patches to make
sure there is no coupling between pblk and lightnvm, but if Matias is ok
with starting having patches covering all at once, then good for me too.

I often object when a patch can logically be split into two and should
be two distinct parts. In this case, it fits together.