Re: [PATCH 5/5] lightnvm: pblk: Disable interleaved metadata in pblk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 18.06.2018 07:29, Javier Gonzalez wrote:
On 16 Jun 2018, at 21.38, Matias Bjørling <mb@xxxxxxxxxxx> wrote:

On 06/16/2018 12:27 AM, Igor Konopko wrote:
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 <igor.j.konopko@xxxxxxxxx>
---
  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
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,
  		return ERR_PTR(-EINVAL);
  	}
  +	if (geo->ext) {
+		pr_err("pblk: extended (interleaved) metadata in data buffer"
+			" not supported\n");
+		return ERR_PTR(-EINVAL);
+	}
+
  	pblk = 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
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 = 1 << ns->lba_shift;
  	geo->sos = ns->ms;
+	geo->ext = 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, just over the 1.2 stuff?

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.

Javier


Will fix above comments and resend.

Igor



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux