| Message ID | 20190206020453.25534-1-oohall@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 07464e88365e9236febaca9ed1a2e2006d8bc952 |
| Headers | show |
| Series | libnvdimm: Fix altmap reservation size calculation | expand |
On Wed, 2019-02-06 at 13:04 +1100, Oliver O'Halloran wrote: > Libnvdimm reserves the first 8K of pfn and devicedax namespaces to > store a superblock describing the namespace. This 8K reservation > is contained within the altmap area which the kernel uses for the > vmemmap backing for the pages within the namespace. The altmap > allows for some pages at the start of the altmap area to be reserved > and that mechanism is used to protect the superblock from being > re-used as vmemmap backing. > > The number of PFNs to reserve is calculated using: > > PHYS_PFN(SZ_8K) > > Which is implemented as: > > #define PHYS_PFN(x) ((unsigned long)((x) >> PAGE_SHIFT)) > > So on systems where PAGE_SIZE is greater than 8K the reservation > size is truncated to zero and the superblock area is re-used as > vmemmap backing. As a result all the namespace information stored > in the superblock (i.e. if it's a PFN or DAX namespace) is lost > and the namespace needs to be re-created to get access to the > contents. > > This patch fixes this by using PFN_UP() rather than PHYS_PFN() to ensure > that at least one page is reserved. On systems with a 4K pages size this > patch should have no effect. > > Cc: stable@vger.kernel.org > Cc: Dan Williams <dan.j.williams@intel.com> > Fixes: ac515c084be9 ("libnvdimm, pmem, pfn: move pfn setup to the core") > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > --- > drivers/nvdimm/pfn_devs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Looks good to me, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index 6f22272e8d80..9b9be83da0e7 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -593,7 +593,7 @@ static unsigned long init_altmap_base(resource_size_t base) > > static unsigned long init_altmap_reserve(resource_size_t base) > { > - unsigned long reserve = PHYS_PFN(SZ_8K); > + unsigned long reserve = PFN_UP(SZ_8K); > unsigned long base_pfn = PHYS_PFN(base); > > reserve += base_pfn - PFN_SECTION_ALIGN_DOWN(base_pfn);
On Tue, Feb 5, 2019 at 6:05 PM Oliver O'Halloran <oohall@gmail.com> wrote: > > Libnvdimm reserves the first 8K of pfn and devicedax namespaces to > store a superblock describing the namespace. This 8K reservation > is contained within the altmap area which the kernel uses for the > vmemmap backing for the pages within the namespace. The altmap > allows for some pages at the start of the altmap area to be reserved > and that mechanism is used to protect the superblock from being > re-used as vmemmap backing. > > The number of PFNs to reserve is calculated using: > > PHYS_PFN(SZ_8K) > > Which is implemented as: > > #define PHYS_PFN(x) ((unsigned long)((x) >> PAGE_SHIFT)) > > So on systems where PAGE_SIZE is greater than 8K the reservation > size is truncated to zero and the superblock area is re-used as > vmemmap backing. As a result all the namespace information stored > in the superblock (i.e. if it's a PFN or DAX namespace) is lost > and the namespace needs to be re-created to get access to the > contents. > > This patch fixes this by using PFN_UP() rather than PHYS_PFN() to ensure > that at least one page is reserved. On systems with a 4K pages size this > patch should have no effect. > > Cc: stable@vger.kernel.org > Cc: Dan Williams <dan.j.williams@intel.com> > Fixes: ac515c084be9 ("libnvdimm, pmem, pfn: move pfn setup to the core") > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > --- > drivers/nvdimm/pfn_devs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index 6f22272e8d80..9b9be83da0e7 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -593,7 +593,7 @@ static unsigned long init_altmap_base(resource_size_t base) > > static unsigned long init_altmap_reserve(resource_size_t base) > { > - unsigned long reserve = PHYS_PFN(SZ_8K); > + unsigned long reserve = PFN_UP(SZ_8K); > unsigned long base_pfn = PHYS_PFN(base); Good find. I know how annoying it is to debug altmap issues. I'll get this queued up.
On Tue, Feb 5, 2019 at 6:38 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Tue, Feb 5, 2019 at 6:05 PM Oliver O'Halloran <oohall@gmail.com> wrote: > > > > Libnvdimm reserves the first 8K of pfn and devicedax namespaces to > > store a superblock describing the namespace. This 8K reservation > > is contained within the altmap area which the kernel uses for the > > vmemmap backing for the pages within the namespace. The altmap > > allows for some pages at the start of the altmap area to be reserved > > and that mechanism is used to protect the superblock from being > > re-used as vmemmap backing. > > > > The number of PFNs to reserve is calculated using: > > > > PHYS_PFN(SZ_8K) > > > > Which is implemented as: > > > > #define PHYS_PFN(x) ((unsigned long)((x) >> PAGE_SHIFT)) > > > > So on systems where PAGE_SIZE is greater than 8K the reservation > > size is truncated to zero and the superblock area is re-used as > > vmemmap backing. As a result all the namespace information stored > > in the superblock (i.e. if it's a PFN or DAX namespace) is lost > > and the namespace needs to be re-created to get access to the > > contents. > > > > This patch fixes this by using PFN_UP() rather than PHYS_PFN() to ensure > > that at least one page is reserved. On systems with a 4K pages size this > > patch should have no effect. > > > > Cc: stable@vger.kernel.org > > Cc: Dan Williams <dan.j.williams@intel.com> > > Fixes: ac515c084be9 ("libnvdimm, pmem, pfn: move pfn setup to the core") > > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > > --- > > --- > > drivers/nvdimm/pfn_devs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > > index 6f22272e8d80..9b9be83da0e7 100644 > > --- a/drivers/nvdimm/pfn_devs.c > > +++ b/drivers/nvdimm/pfn_devs.c > > @@ -593,7 +593,7 @@ static unsigned long init_altmap_base(resource_size_t base) > > > > static unsigned long init_altmap_reserve(resource_size_t base) > > { > > - unsigned long reserve = PHYS_PFN(SZ_8K); > > + unsigned long reserve = PFN_UP(SZ_8K); > > unsigned long base_pfn = PHYS_PFN(base); > > Good find. I know how annoying it is to debug altmap issues. > > I'll get this queued up. Hmm, this consideration should also be included in the data_offset calculation. I'll take a look.
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index 6f22272e8d80..9b9be83da0e7 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -593,7 +593,7 @@ static unsigned long init_altmap_base(resource_size_t base) static unsigned long init_altmap_reserve(resource_size_t base) { - unsigned long reserve = PHYS_PFN(SZ_8K); + unsigned long reserve = PFN_UP(SZ_8K); unsigned long base_pfn = PHYS_PFN(base); reserve += base_pfn - PFN_SECTION_ALIGN_DOWN(base_pfn);
Libnvdimm reserves the first 8K of pfn and devicedax namespaces to store a superblock describing the namespace. This 8K reservation is contained within the altmap area which the kernel uses for the vmemmap backing for the pages within the namespace. The altmap allows for some pages at the start of the altmap area to be reserved and that mechanism is used to protect the superblock from being re-used as vmemmap backing. The number of PFNs to reserve is calculated using: PHYS_PFN(SZ_8K) Which is implemented as: #define PHYS_PFN(x) ((unsigned long)((x) >> PAGE_SHIFT)) So on systems where PAGE_SIZE is greater than 8K the reservation size is truncated to zero and the superblock area is re-used as vmemmap backing. As a result all the namespace information stored in the superblock (i.e. if it's a PFN or DAX namespace) is lost and the namespace needs to be re-created to get access to the contents. This patch fixes this by using PFN_UP() rather than PHYS_PFN() to ensure that at least one page is reserved. On systems with a 4K pages size this patch should have no effect. Cc: stable@vger.kernel.org Cc: Dan Williams <dan.j.williams@intel.com> Fixes: ac515c084be9 ("libnvdimm, pmem, pfn: move pfn setup to the core") Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- --- drivers/nvdimm/pfn_devs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)