Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, 28 Jun 2019 12:14:44 -0700 Dan Williams wrote: > I believe -mm auto drops patches when they appear in the -next > baseline. So it should "just work" to pull it into the series and send > it along for -next inclusion. Yup. Although it isn't very "auto" - I manually check that the patch which turned up in -next was identical to the version which I had. If not, I go find out why... ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 12:02 PM Christoph Hellwig wrote: > > On Fri, Jun 28, 2019 at 11:59:19AM -0700, Dan Williams wrote: > > It's a bug that the call to put_devmap_managed_page() was gated by > > MEMORY_DEVICE_PUBLIC in release_pages(). That path is also applicable > > to MEMORY_DEVICE_FSDAX because it needs to trigger the ->page_free() > > callback to wake up wait_on_var() via fsdax_pagefree(). > > > > So I guess you could argue that the MEMORY_DEVICE_PUBLIC removal patch > > left the original bug in place. In that sense we're no worse off, but > > since we know about the bug, the fix and the patches have not been > > applied yet, why not fix it now? > > The fix it now would simply be to apply Ira original patch now, but > given that we are at -rc6 is this really a good time? And if we don't > apply it now based on the quilt based -mm worflow it just seems a lot > easier to apply it after my series. Unless we want to include it in > the series, in which case I can do a quick rebase, we'd just need to > make sure Andrew pulls it from -mm. I believe -mm auto drops patches when they appear in the -next baseline. So it should "just work" to pull it into the series and send it along for -next inclusion. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 11:59:19AM -0700, Dan Williams wrote: > It's a bug that the call to put_devmap_managed_page() was gated by > MEMORY_DEVICE_PUBLIC in release_pages(). That path is also applicable > to MEMORY_DEVICE_FSDAX because it needs to trigger the ->page_free() > callback to wake up wait_on_var() via fsdax_pagefree(). > > So I guess you could argue that the MEMORY_DEVICE_PUBLIC removal patch > left the original bug in place. In that sense we're no worse off, but > since we know about the bug, the fix and the patches have not been > applied yet, why not fix it now? The fix it now would simply be to apply Ira original patch now, but given that we are at -rc6 is this really a good time? And if we don't apply it now based on the quilt based -mm worflow it just seems a lot easier to apply it after my series. Unless we want to include it in the series, in which case I can do a quick rebase, we'd just need to make sure Andrew pulls it from -mm. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 09:27:44AM -0700, Dan Williams wrote: > On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe wrote: > > > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote: > > > The functionality is identical to the one currently open coded in > > > device-dax. > > > > > > Signed-off-by: Christoph Hellwig > > > Reviewed-by: Ira Weiny > > > drivers/dax/dax-private.h | 4 > > > drivers/dax/device.c | 43 --- > > > 2 files changed, 47 deletions(-) > > > > DanW: I think this series has reached enough review, did you want > > to ack/test any further? > > > > This needs to land in hmm.git soon to make the merge window. > > I was awaiting a decision about resolving the collision with Ira's > patch before testing the final result again [1]. You can go ahead and > add my reviewed-by for the series, but my tested-by should be on the > final state of the series. The conflict looks OK to me, I think we can let Andrew and Linus resolve it. Thanks, Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 8:39 AM Jason Gunthorpe wrote: > > On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote: > > The functionality is identical to the one currently open coded in > > device-dax. > > > > Signed-off-by: Christoph Hellwig > > Reviewed-by: Ira Weiny > > --- > > drivers/dax/dax-private.h | 4 > > drivers/dax/device.c | 43 --- > > 2 files changed, 47 deletions(-) > > DanW: I think this series has reached enough review, did you want > to ack/test any further? > > This needs to land in hmm.git soon to make the merge window. I was awaiting a decision about resolving the collision with Ira's patch before testing the final result again [1]. You can go ahead and add my reviewed-by for the series, but my tested-by should be on the final state of the series. [1]: https://lore.kernel.org/lkml/CAPcyv4gTOf+EWzSGrFrh2GrTZt5HVR=e+xicukepiy57px8...@mail.gmail.com/ ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote: > The functionality is identical to the one currently open coded in > device-dax. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Ira Weiny > --- > drivers/dax/dax-private.h | 4 > drivers/dax/device.c | 43 --- > 2 files changed, 47 deletions(-) DanW: I think this series has reached enough review, did you want to ack/test any further? This needs to land in hmm.git soon to make the merge window. Thanks, Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Wed, Jun 26, 2019 at 02:27:15PM +0200, Christoph Hellwig wrote: > The functionality is identical to the one currently open coded in > device-dax. > > Signed-off-by: Christoph Hellwig Reviewed-by: Ira Weiny > --- > drivers/dax/dax-private.h | 4 > drivers/dax/device.c | 43 --- > 2 files changed, 47 deletions(-) > > diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h > index b4177aafbbd1..c915889d1769 100644 > --- a/drivers/dax/dax-private.h > +++ b/drivers/dax/dax-private.h > @@ -43,8 +43,6 @@ struct dax_region { > * @target_node: effective numa node if dev_dax memory range is onlined > * @dev - device core > * @pgmap - pgmap for memmap setup / lifetime (driver owned) > - * @ref: pgmap reference count (driver owned) > - * @cmp: @ref final put completion (driver owned) > */ > struct dev_dax { > struct dax_region *region; > @@ -52,8 +50,6 @@ struct dev_dax { > int target_node; > struct device dev; > struct dev_pagemap pgmap; > - struct percpu_ref ref; > - struct completion cmp; > }; > > static inline struct dev_dax *to_dev_dax(struct device *dev) > diff --git a/drivers/dax/device.c b/drivers/dax/device.c > index b5257038c188..1af823b2fe6b 100644 > --- a/drivers/dax/device.c > +++ b/drivers/dax/device.c > @@ -14,36 +14,6 @@ > #include "dax-private.h" > #include "bus.h" > > -static struct dev_dax *ref_to_dev_dax(struct percpu_ref *ref) > -{ > - return container_of(ref, struct dev_dax, ref); > -} > - > -static void dev_dax_percpu_release(struct percpu_ref *ref) > -{ > - struct dev_dax *dev_dax = ref_to_dev_dax(ref); > - > - dev_dbg(&dev_dax->dev, "%s\n", __func__); > - complete(&dev_dax->cmp); > -} > - > -static void dev_dax_percpu_exit(struct dev_pagemap *pgmap) > -{ > - struct dev_dax *dev_dax = container_of(pgmap, struct dev_dax, pgmap); > - > - dev_dbg(&dev_dax->dev, "%s\n", __func__); > - wait_for_completion(&dev_dax->cmp); > - percpu_ref_exit(pgmap->ref); > -} > - > -static void dev_dax_percpu_kill(struct dev_pagemap *pgmap) > -{ > - struct dev_dax *dev_dax = container_of(pgmap, struct dev_dax, pgmap); > - > - dev_dbg(&dev_dax->dev, "%s\n", __func__); > - percpu_ref_kill(pgmap->ref); > -} > - > static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma, > const char *func) > { > @@ -441,11 +411,6 @@ static void dev_dax_kill(void *dev_dax) > kill_dev_dax(dev_dax); > } > > -static const struct dev_pagemap_ops dev_dax_pagemap_ops = { > - .kill = dev_dax_percpu_kill, > - .cleanup= dev_dax_percpu_exit, > -}; > - > int dev_dax_probe(struct device *dev) > { > struct dev_dax *dev_dax = to_dev_dax(dev); > @@ -463,15 +428,7 @@ int dev_dax_probe(struct device *dev) > return -EBUSY; > } > > - init_completion(&dev_dax->cmp); > - rc = percpu_ref_init(&dev_dax->ref, dev_dax_percpu_release, 0, > - GFP_KERNEL); > - if (rc) > - return rc; > - > - dev_dax->pgmap.ref = &dev_dax->ref; > dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX; > - dev_dax->pgmap.ops = &dev_dax_pagemap_ops; > addr = devm_memremap_pages(dev, &dev_dax->pgmap); > if (IS_ERR(addr)) > return PTR_ERR(addr); > -- > 2.20.1 > > ___ > Linux-nvdimm mailing list > linux-nvd...@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau