hello

On Fri, Jan 10, 2020 at 11:05 AM <[email protected]> wrote:

> Send Linux-nvdimm mailing list submissions to
>         [email protected]
>
> To subscribe or unsubscribe via email, send a message with subject or
> body 'help' to
>         [email protected]
>
> You can reach the person managing the list at
>         [email protected]
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Linux-nvdimm digest..."
>
> Today's Topics:
>
>    1. [PATCH ndctl] ndctl/namespace: Fix enable-namespace error for seed
> namespaces
>       (Santosh Sivaraj)
>    2. Re: dax: Get rid of fs_dax_get_by_host() helper
>       (Christoph Hellwig)
>    3. Re: [PATCH 01/19] dax: remove block device dependencies
>       (Christoph Hellwig)
>    4. Re: [PATCH ndctl] ndctl/namespace: Fix enable-namespace error for
> seed namespaces
>       (Dan Williams)
>    5. [PATCH RFC 06/10] device-dax: Introduce pfn_flags helper
>       (Joao Martins)
>    6. [PATCH RFC 07/10] device-dax: Add support for PFN_SPECIAL flags
>       (Joao Martins)
>
>
> ----------------------------------------------------------------------
>
> Date: Fri, 10 Jan 2020 13:50:17 +0530
> From: Santosh Sivaraj <[email protected]>
> Subject: [PATCH ndctl] ndctl/namespace: Fix enable-namespace error for
>         seed namespaces
> To: [email protected],  Dan Williams
>         <[email protected]>
> Cc: [email protected]
> Message-ID: <[email protected]>
>
> 'ndctl enable-namespace all' tries to enable seed namespaces too, which
> results
> in a error like
>
> libndctl: ndctl_namespace_enable: namespace1.0: failed to enable
>
> Signed-off-by: Santosh Sivaraj <[email protected]>
> ---
>  ndctl/lib/libndctl.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 6596f94..4839214 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -4010,11 +4010,16 @@ NDCTL_EXPORT int ndctl_namespace_enable(struct
> ndctl_namespace *ndns)
>         const char *devname = ndctl_namespace_get_devname(ndns);
>         struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
>         struct ndctl_region *region = ndns->region;
> +       unsigned long long size = ndctl_namespace_get_size(ndns);
>         int rc;
>
>         if (ndctl_namespace_is_enabled(ndns))
>                 return 0;
>
> +       /* Don't try to enable idle namespace (no capacity allocated) */
> +       if (size == 0)
> +               return -1;
> +
>         rc = ndctl_bind(ctx, ndns->module, devname);
>
>         /*
> --
> 2.24.1
>
> ------------------------------
>
> Date: Fri, 10 Jan 2020 04:31:27 -0800
> From: Christoph Hellwig <[email protected]>
> Subject: Re: dax: Get rid of fs_dax_get_by_host() helper
> To: Vivek Goyal <[email protected]>
> Cc: [email protected], [email protected],
>         [email protected]
> Message-ID: <[email protected]>
> Content-Type: text/plain; charset=us-ascii
>
> On Mon, Jan 06, 2020 at 01:11:17PM -0500, Vivek Goyal wrote:
> > Looks like nobody is using fs_dax_get_by_host() except
> fs_dax_get_by_bdev()
> > and it can easily use dax_get_by_host() instead.
> >
> > IIUC, fs_dax_get_by_host() was only introduced so that one could compile
> > with CONFIG_FS_DAX=n and CONFIG_DAX=m. fs_dax_get_by_bdev() achieves
> > the same purpose and hence it looks like fs_dax_get_by_host() is not
> > needed anymore.
> >
> > Signed-off-by: Vivek Goyal <[email protected]>
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> ------------------------------
>
> Date: Fri, 10 Jan 2020 04:36:31 -0800
> From: Christoph Hellwig <[email protected]>
> Subject: Re: [PATCH 01/19] dax: remove block device dependencies
> To: Dan Williams <[email protected]>
> Cc: Jan Kara <[email protected]>, "Darrick J. Wong"
>         <[email protected]>, Christoph Hellwig <[email protected]>,
> Dave
>         Chinner <[email protected]>, Miklos Szeredi <[email protected]>,
>         linux-nvdimm <[email protected]>, Linux Kernel Mailing
> List
>         <[email protected]>, "Dr.  David Alan Gilbert"
>         <[email protected]>, [email protected], Stefan Hajnoczi
>         <[email protected]>, linux-fsdevel <
> [email protected]>
> Message-ID: <[email protected]>
> Content-Type: text/plain; charset=us-ascii
>
> On Thu, Jan 09, 2020 at 12:03:01PM -0800, Dan Williams wrote:
> > > So I'd find two options reasonably consistent:
> > > 1) Keep status quo where partitions are created and support DAX.
> > > 2) Stop partition creation altogether, if anyones wants to split pmem
> > > device further, he can use dm-linear for that (i.e., kpartx).
> > >
> > > But I'm not sure if the ship hasn't already sailed for option 2) to be
> > > feasible without angry users and Linus reverting the change.
> >
> > Christoph? I feel myself leaning more and more to the "keep pmem
> > partitions" camp.
> >
> > I don't see "drop partition support" effort ending well given the long
> > standing "ext4 fails to mount when dax is not available" precedent.
>
> Do we have any evidence of existing setups with DAX and partitions?
> Can we just throw in a patch to reject that case for now before actually
> removing the code and see if anyone screams.  And fix ext4 up while
> we are at it.
>
> > I think the next least bad option is to have a dax_get_by_host()
> > variant that passes an offset and length pair rather than requiring a
> > later bdev_dax_pgoff() to recall the offset. This also prevents
> > needing to add another dax-device object representation.
>
> IFF we have to keep partition support, yes.  But keeping it just seems
> like a really bad idea.
>
> ------------------------------
>
> Date: Fri, 10 Jan 2020 09:56:26 -0800
> From: Dan Williams <[email protected]>
> Subject: Re: [PATCH ndctl] ndctl/namespace: Fix enable-namespace error
>         for seed namespaces
> To: Santosh Sivaraj <[email protected]>
> Cc: linux-nvdimm <[email protected]>, [email protected]
> Message-ID:
>         <
> capcyv4h-ke4jorqx6md+gfgvupngxn-qm8yx7valna4o+91...@mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> On Fri, Jan 10, 2020 at 12:21 AM Santosh Sivaraj <[email protected]>
> wrote:
> >
> > 'ndctl enable-namespace all' tries to enable seed namespaces too, which
> results
> > in a error like
> >
> > libndctl: ndctl_namespace_enable: namespace1.0: failed to enable
> >
> > Signed-off-by: Santosh Sivaraj <[email protected]>
> > ---
> >  ndctl/lib/libndctl.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index 6596f94..4839214 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -4010,11 +4010,16 @@ NDCTL_EXPORT int ndctl_namespace_enable(struct
> ndctl_namespace *ndns)
> >         const char *devname = ndctl_namespace_get_devname(ndns);
> >         struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> >         struct ndctl_region *region = ndns->region;
> > +       unsigned long long size = ndctl_namespace_get_size(ndns);
> >         int rc;
> >
> >         if (ndctl_namespace_is_enabled(ndns))
> >                 return 0;
> >
> > +       /* Don't try to enable idle namespace (no capacity allocated) */
> > +       if (size == 0)
> > +               return -1;
>
> Concept looks good to me, just resend with that -1 changed to a named
> error code (-ENXIO).
>
> ------------------------------
>
> Date: Fri, 10 Jan 2020 19:03:09 +0000
> From: Joao Martins <[email protected]>
> Subject: [PATCH RFC 06/10] device-dax: Introduce pfn_flags helper
> To: [email protected]
> Cc: Alex Williamson <[email protected]>, Cornelia Huck
>         <[email protected]>, [email protected], Andrew Morton
>         <[email protected]>, [email protected],
>         [email protected], Thomas Gleixner <[email protected]
> >,
>         Ingo Molnar <[email protected]>, Borislav Petkov <[email protected]>,
> "H .
>         Peter Anvin" <[email protected]>, [email protected], Liran Alon
>         <[email protected]>, Nikita Leshenko
>         <[email protected]>, Barret Rhoden <[email protected]>,
> Boris
>         Ostrovsky <[email protected]>, Matthew Wilcox
>         <[email protected]>, Konrad Rzeszutek Wilk <
> [email protected]>
> Message-ID: <[email protected]>
>
> Replace PFN_DEV|PFN_MAP check call sites with two helper functions
> dax_is_pfn_dev() and dax_is_pfn_map().
>
> Signed-off-by: Joao Martins <[email protected]>
> ---
>  drivers/dax/device.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index c6a7f5e12c54..113a554de3ee 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -14,6 +14,17 @@
>  #include "dax-private.h"
>  #include "bus.h"
>
> +static int dax_is_pfn_dev(struct dev_dax *dev_dax)
> +{
> +       return (dev_dax->region->pfn_flags & (PFN_DEV|PFN_MAP)) == PFN_DEV;
> +}
> +
> +static int dax_is_pfn_map(struct dev_dax *dev_dax)
> +{
> +       return (dev_dax->region->pfn_flags &
> +               (PFN_DEV|PFN_MAP)) == (PFN_DEV|PFN_MAP);
> +}
> +
>  static int check_vma_mmap(struct dev_dax *dev_dax, struct vm_area_struct
> *vma,
>                 const char *func)
>  {
> @@ -60,8 +71,7 @@ static int check_vma(struct dev_dax *dev_dax, struct
> vm_area_struct *vma,
>         if (rc < 0)
>                 return rc;
>
> -       if ((dev_dax->region->pfn_flags & (PFN_DEV|PFN_MAP)) == PFN_DEV
> -                       && (vma->vm_flags & VM_DONTCOPY) == 0) {
> +       if (dax_is_pfn_dev(dev_dax) && (vma->vm_flags & VM_DONTCOPY) == 0)
> {
>                 dev_info_ratelimited(&dev_dax->dev,
>                                 "%s: %s: fail, dax range requires
> MADV_DONTFORK\n",
>                                 current->comm, func);
> @@ -140,7 +150,7 @@ static vm_fault_t __dev_dax_pmd_fault(struct dev_dax
> *dev_dax,
>         }
>
>         /* dax pmd mappings require pfn_t_devmap() */
> -       if ((dax_region->pfn_flags & (PFN_DEV|PFN_MAP)) !=
> (PFN_DEV|PFN_MAP)) {
> +       if (!dax_is_pfn_map(dev_dax)) {
>                 dev_dbg(dev, "region lacks devmap flags\n");
>                 return VM_FAULT_SIGBUS;
>         }
> @@ -190,7 +200,7 @@ static vm_fault_t __dev_dax_pud_fault(struct dev_dax
> *dev_dax,
>         }
>
>         /* dax pud mappings require pfn_t_devmap() */
> -       if ((dax_region->pfn_flags & (PFN_DEV|PFN_MAP)) !=
> (PFN_DEV|PFN_MAP)) {
> +       if (!dax_is_pfn_map(dev_dax)) {
>                 dev_dbg(dev, "region lacks devmap flags\n");
>                 return VM_FAULT_SIGBUS;
>         }
> --
> 2.17.1
>
> ------------------------------
>
> Date: Fri, 10 Jan 2020 19:03:10 +0000
> From: Joao Martins <[email protected]>
> Subject: [PATCH RFC 07/10] device-dax: Add support for PFN_SPECIAL
>         flags
> To: [email protected]
> Cc: Alex Williamson <[email protected]>, Cornelia Huck
>         <[email protected]>, [email protected], Andrew Morton
>         <[email protected]>, [email protected],
>         [email protected], Thomas Gleixner <[email protected]
> >,
>         Ingo Molnar <[email protected]>, Borislav Petkov <[email protected]>,
> "H .
>         Peter Anvin" <[email protected]>, [email protected], Liran Alon
>         <[email protected]>, Nikita Leshenko
>         <[email protected]>, Barret Rhoden <[email protected]>,
> Boris
>         Ostrovsky <[email protected]>, Matthew Wilcox
>         <[email protected]>, Konrad Rzeszutek Wilk <
> [email protected]>
> Message-ID: <[email protected]>
>
> Right now we assume there's gonna be a PFN_DEV|PFN_MAP which
> means it will have a struct page backing the PFN but that is
> not placed in normal system RAM zones.
>
> Add support for PFN_DEV|PFN_SPECIAL only and therefore the
> underlying vma won't have a struct page. For device dax, this
> means not assuming callers will pass a dev_pagemap, and avoid
> returning SIGBUS for the lack of PFN_MAP region pfn flag and
> finally not setting struct page index/mapping on fault.
>
> Signed-off-by: Joao Martins <[email protected]>
> ---
>  drivers/dax/bus.c    |  3 ++-
>  drivers/dax/device.c | 40 ++++++++++++++++++++++------------------
>  2 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 46e46047a1f7..96ca3ac85278 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -414,7 +414,8 @@ struct dev_dax *__devm_create_dev_dax(struct
> dax_region *dax_region, int id,
>         if (!dev_dax)
>                 return ERR_PTR(-ENOMEM);
>
> -       memcpy(&dev_dax->pgmap, pgmap, sizeof(*pgmap));
> +       if (pgmap)
> +               memcpy(&dev_dax->pgmap, pgmap, sizeof(*pgmap));
>
>         /*
>          * No 'host' or dax_operations since there is no access to this
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index 113a554de3ee..aa38f5ff180a 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -14,6 +14,12 @@
>  #include "dax-private.h"
>  #include "bus.h"
>
> +static int dax_is_pfn_special(struct dev_dax *dev_dax)
> +{
> +       return (dev_dax->region->pfn_flags &
> +               (PFN_DEV|PFN_SPECIAL)) == (PFN_DEV|PFN_SPECIAL);
> +}
> +
>  static int dax_is_pfn_dev(struct dev_dax *dev_dax)
>  {
>         return (dev_dax->region->pfn_flags & (PFN_DEV|PFN_MAP)) == PFN_DEV;
> @@ -104,6 +110,7 @@ static vm_fault_t __dev_dax_pte_fault(struct dev_dax
> *dev_dax,
>         struct dax_region *dax_region;
>         phys_addr_t phys;
>         unsigned int fault_size = PAGE_SIZE;
> +       int rc;
>
>         if (check_vma(dev_dax, vmf->vma, __func__))
>                 return VM_FAULT_SIGBUS;
> @@ -126,7 +133,12 @@ static vm_fault_t __dev_dax_pte_fault(struct dev_dax
> *dev_dax,
>
>         *pfn = phys_to_pfn_t(phys, dax_region->pfn_flags);
>
> -       return vmf_insert_mixed(vmf->vma, vmf->address, *pfn);
> +       if (dax_is_pfn_special(dev_dax))
> +               rc = vmf_insert_pfn(vmf->vma, vmf->address,
> pfn_t_to_pfn(*pfn));
> +       else
> +               rc = vmf_insert_mixed(vmf->vma, vmf->address, *pfn);
> +
> +       return rc;
>  }
>
>  static vm_fault_t __dev_dax_pmd_fault(struct dev_dax *dev_dax,
> @@ -149,12 +161,6 @@ static vm_fault_t __dev_dax_pmd_fault(struct dev_dax
> *dev_dax,
>                 return VM_FAULT_SIGBUS;
>         }
>
> -       /* dax pmd mappings require pfn_t_devmap() */
> -       if (!dax_is_pfn_map(dev_dax)) {
> -               dev_dbg(dev, "region lacks devmap flags\n");
> -               return VM_FAULT_SIGBUS;
> -       }
> -
>         if (fault_size < dax_region->align)
>                 return VM_FAULT_SIGBUS;
>         else if (fault_size > dax_region->align)
> @@ -199,12 +205,6 @@ static vm_fault_t __dev_dax_pud_fault(struct dev_dax
> *dev_dax,
>                 return VM_FAULT_SIGBUS;
>         }
>
> -       /* dax pud mappings require pfn_t_devmap() */
> -       if (!dax_is_pfn_map(dev_dax)) {
> -               dev_dbg(dev, "region lacks devmap flags\n");
> -               return VM_FAULT_SIGBUS;
> -       }
> -
>         if (fault_size < dax_region->align)
>                 return VM_FAULT_SIGBUS;
>         else if (fault_size > dax_region->align)
> @@ -266,7 +266,7 @@ static vm_fault_t dev_dax_huge_fault(struct vm_fault
> *vmf,
>                 rc = VM_FAULT_SIGBUS;
>         }
>
> -       if (rc == VM_FAULT_NOPAGE) {
> +       if (dax_is_pfn_map(dev_dax) && (rc == VM_FAULT_NOPAGE)) {
>                 unsigned long i;
>                 pgoff_t pgoff;
>
> @@ -344,6 +344,8 @@ static int dax_mmap(struct file *filp, struct
> vm_area_struct *vma)
>
>         vma->vm_ops = &dax_vm_ops;
>         vma->vm_flags |= VM_HUGEPAGE;
> +       if (dax_is_pfn_special(dev_dax))
> +               vma->vm_flags |= VM_PFNMAP;
>         return 0;
>  }
>
> @@ -450,10 +452,12 @@ int dev_dax_probe(struct device *dev)
>                 return -EBUSY;
>         }
>
> -       dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> -       addr = devm_memremap_pages(dev, &dev_dax->pgmap);
> -       if (IS_ERR(addr))
> -               return PTR_ERR(addr);
> +       if (dax_is_pfn_map(dev_dax)) {
> +               dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> +               addr = devm_memremap_pages(dev, &dev_dax->pgmap);
> +               if (IS_ERR(addr))
> +                       return PTR_ERR(addr);
> +       }
>
>         inode = dax_inode(dax_dev);
>         cdev = inode->i_cdev;
> --
> 2.17.1
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> Linux-nvdimm mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
>
>
> ------------------------------
>
> End of Linux-nvdimm Digest, Vol 64, Issue 18
> ********************************************
>
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to