On Tue, Feb 5, 2019 at 6:38 PM Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> On Tue, Feb 5, 2019 at 6:05 PM Oliver O'Halloran <oohall@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> > Fixes: ac515c084be9 ("libnvdimm, pmem, pfn: move pfn setup to the core")
> > Signed-off-by: Oliver O'Halloran <oohall@xxxxxxxxx>
> > ---
> > ---
> > 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.
