Dan Williams
2019-Jun-28 18:59 UTC
[Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 11:52 AM Christoph Hellwig <hch at lst.de> wrote:> > On Fri, Jun 28, 2019 at 11:44:35AM -0700, Dan Williams wrote: > > There is a problem with the series in CH's tree. It removes the > > ->page_free() callback from the release_pages() path because it goes > > too far and removes the put_devmap_managed_page() call. > > release_pages only called put_devmap_managed_page for device public > pages. So I can't see how that is in any way a problem.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?
Christoph Hellwig
2019-Jun-28 19:02 UTC
[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.
Dan Williams
2019-Jun-28 19:14 UTC
[Nouveau] [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
On Fri, Jun 28, 2019 at 12:02 PM Christoph Hellwig <hch at lst.de> 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.
Possibly Parallel Threads
- [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
- [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
- [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
- [PATCH 16/25] device-dax: use the dev_pagemap internal refcount
- [PATCH 08/25] memremap: move dev_pagemap callbacks into a separate structure