On Mon, Jun 18, 2018 at 4:29 PM, Javier Gonzalez <javier@xxxxxxxxxxxx> 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.
>
I often object when a patch can logically be split into two and should
be two distinct parts. In this case, it fits together.
