| Message ID | 20190809074520.27115-3-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Mark the namespace disabled on pfn superblock mismatch | expand |
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: case PFN_MODE_PMEM: > @@ -475,6 +484,20 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) > align = 1UL << ilog2(offset); > mode = le32_to_cpu(pfn_sb->mode); > > + if (le32_to_cpu(pfn_sb->page_size) != PAGE_SIZE) { > + dev_err(&nd_pfn->dev, > + "init failed, page size mismatch %d\n", > + le32_to_cpu(pfn_sb->page_size)); > + return -EOPNOTSUPP; > + } > + > + if (le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) { > + dev_err(&nd_pfn->dev, > + "init failed, struct page size mismatch %d\n", > + le16_to_cpu(pfn_sb->page_struct_size)); > + return -EOPNOTSUPP; > + } > + We need this here? From 9885b2f9ed81a2438fc81507cfcdbdb1aeab756c Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> Date: Fri, 9 Aug 2019 22:10:08 +0530 Subject: [PATCH] nvdimm: check struct page size only if pfn node is PMEM We should do the check only with PFN_MODE_PMEM. If we use memory for backing vmemmap, we should be able to enable the namespace even if struct page size change. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- drivers/nvdimm/pfn_devs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index f43d1baa6f33..f3e9a4b826da 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -509,7 +509,8 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) return -EOPNOTSUPP; } - if (le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) { + if ((le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) && + (mode == PFN_MODE_PMEM)) { dev_err(&nd_pfn->dev, "init failed, struct page size mismatch %d\n", le16_to_cpu(pfn_sb->page_struct_size));
On Fri, Aug 9, 2019 at 9:21 PM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > > case PFN_MODE_PMEM: > > @@ -475,6 +484,20 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) > > align = 1UL << ilog2(offset); > > mode = le32_to_cpu(pfn_sb->mode); > > > > + if (le32_to_cpu(pfn_sb->page_size) != PAGE_SIZE) { I think this is too strict. It's only a potential problem in the "pfn_sb->page_size > PAGE_SIZE" case, because only then might the existing reservation for the memmap be too small. Otherwise, unless I'm missing something, there should be no ill effects for under-utilizing the reservation. > > + dev_err(&nd_pfn->dev, > > + "init failed, page size mismatch %d\n", > > + le32_to_cpu(pfn_sb->page_size)); > > + return -EOPNOTSUPP; > > + } > > + > > + if (le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) { > > + dev_err(&nd_pfn->dev, > > + "init failed, struct page size mismatch %d\n", > > + le16_to_cpu(pfn_sb->page_struct_size)); > > + return -EOPNOTSUPP; > > + } > > + > > We need this here? Yes, both ->page_struct_size and ->page_size are only relevant in the PFN_MODE_PMEM case because PFN_MODE_RAM assumes all pages are allocated dynamically and the size does not matter. > > From 9885b2f9ed81a2438fc81507cfcdbdb1aeab756c Mon Sep 17 00:00:00 2001 > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> > Date: Fri, 9 Aug 2019 22:10:08 +0530 > Subject: [PATCH] nvdimm: check struct page size only if pfn node is PMEM > > We should do the check only with PFN_MODE_PMEM. If we use > memory for backing vmemmap, we should be able to enable > the namespace even if struct page size change. Same as the other patches please drop the usage of "we" in the changelog.
diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h index 7381673b7b70..acb19517f678 100644 --- a/drivers/nvdimm/pfn.h +++ b/drivers/nvdimm/pfn.h @@ -29,7 +29,10 @@ struct nd_pfn_sb { /* minor-version-2 record the base alignment of the mapping */ __le32 align; /* minor-version-3 guarantee the padding and flags are zero */ - u8 padding[4000]; + /* minor-version-4 record the page size and struct page size */ + __le32 page_size; + __le16 page_struct_size; + u8 padding[3994]; __le64 checksum; }; diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index 3e7b11cf1aae..37e96811c2fc 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -460,6 +460,15 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) if (__le16_to_cpu(pfn_sb->version_minor) < 2) pfn_sb->align = 0; + if (__le16_to_cpu(pfn_sb->version_minor) < 4) { + /* + * For a large part we use PAGE_SIZE. But we + * do have some accounting code using SZ_4K. + */ + pfn_sb->page_struct_size = cpu_to_le16(64); + pfn_sb->page_size = cpu_to_le32(PAGE_SIZE); + } + switch (le32_to_cpu(pfn_sb->mode)) { case PFN_MODE_RAM: case PFN_MODE_PMEM: @@ -475,6 +484,20 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) align = 1UL << ilog2(offset); mode = le32_to_cpu(pfn_sb->mode); + if (le32_to_cpu(pfn_sb->page_size) != PAGE_SIZE) { + dev_err(&nd_pfn->dev, + "init failed, page size mismatch %d\n", + le32_to_cpu(pfn_sb->page_size)); + return -EOPNOTSUPP; + } + + if (le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) { + dev_err(&nd_pfn->dev, + "init failed, struct page size mismatch %d\n", + le16_to_cpu(pfn_sb->page_struct_size)); + return -EOPNOTSUPP; + } + if (!nd_pfn->uuid) { /* * When probing a namepace via nd_pfn_probe() the uuid @@ -722,8 +745,10 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn) memcpy(pfn_sb->uuid, nd_pfn->uuid, 16); memcpy(pfn_sb->parent_uuid, nd_dev_to_uuid(&ndns->dev), 16); pfn_sb->version_major = cpu_to_le16(1); - pfn_sb->version_minor = cpu_to_le16(3); + pfn_sb->version_minor = cpu_to_le16(4); pfn_sb->align = cpu_to_le32(nd_pfn->align); + pfn_sb->page_struct_size = cpu_to_le16(sizeof(struct page)); + pfn_sb->page_size = cpu_to_le32(PAGE_SIZE); checksum = nd_sb_checksum((struct nd_gen_sb *) pfn_sb); pfn_sb->checksum = cpu_to_le64(checksum);
This is needed so that we don't wrongly initialize a namespace which doesn't have enough space reserved for holding struct pages with the current kernel. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- drivers/nvdimm/pfn.h | 5 ++++- drivers/nvdimm/pfn_devs.c | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-)