| Message ID | 20241216155408.8102-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dax: Remove access to page->index | expand |
On 12/16/2024 7:53 AM, Matthew Wilcox (Oracle) wrote: > This looks like a complete mess (why are we setting page->index at page > fault time?), but I no longer care about DAX, and there's no reason to > let DAX hold us back from removing page->index. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > drivers/dax/device.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/dax/device.c b/drivers/dax/device.c > index 6d74e62bbee0..bc871a34b9cd 100644 > --- a/drivers/dax/device.c > +++ b/drivers/dax/device.c > @@ -89,14 +89,13 @@ static void dax_set_mapping(struct vm_fault *vmf, pfn_t pfn, > ALIGN_DOWN(vmf->address, fault_size)); > > for (i = 0; i < nr_pages; i++) { > - struct page *page = pfn_to_page(pfn_t_to_pfn(pfn) + i); > + struct folio *folio = pfn_folio(pfn_t_to_pfn(pfn) + i); > > - page = compound_head(page); > - if (page->mapping) > + if (folio->mapping) > continue; > > - page->mapping = filp->f_mapping; > - page->index = pgoff + i; > + folio->mapping = filp->f_mapping; > + folio->index = pgoff + i; > } > } > Looks good. Reviewed-by: Jane Chu <jane.chu@oracle.com> -jane
Matthew Wilcox (Oracle) wrote: > This looks like a complete mess (why are we setting page->index at page > fault time?) Full story in Alistair's patches, but this a side effect of bypassing the page allocator for instantiating file-backed mappings. > but I no longer care about DAX, and there's no reason to > let DAX hold us back from removing page->index. Question is whether to move ahead with this now and have Alistair rebase, or push ahead with getting Alistair's series into -next? I am hoping that Alistair's series can move ahead this cycle, but still catching up on the latest after the holiday break.
On Mon, Jan 06, 2025 at 04:43:13PM -0800, Dan Williams wrote: > Matthew Wilcox (Oracle) wrote: > > This looks like a complete mess (why are we setting page->index at page > > fault time?) > > Full story in Alistair's patches, but this a side effect of bypassing > the page allocator for instantiating file-backed mappings. > > > but I no longer care about DAX, and there's no reason to > > let DAX hold us back from removing page->index. > > Question is whether to move ahead with this now and have Alistair > rebase, or push ahead with getting Alistair's series into -next? I am > hoping that Alistair's series can move ahead this cycle, but still > catching up on the latest after the holiday break. The rebase probably isn't that hard, but if we push ahead with my series it's largely unnecessary as it moves this over to the folio anyway. I've just posted a respin on top of next-20241216 - https://lore.kernel.org/linux-mm/cover.425da7c4e76c2749d0ad1734f972b06114e02d52.1736221254.git-series.apopple@nvidia.com/ - Alistair
On Wed, Jan 08, 2025 at 10:24:00AM +1100, Alistair Popple wrote: > On Mon, Jan 06, 2025 at 04:43:13PM -0800, Dan Williams wrote: > > Matthew Wilcox (Oracle) wrote: > > > This looks like a complete mess (why are we setting page->index at page > > > fault time?) > > > > Full story in Alistair's patches, but this a side effect of bypassing > > the page allocator for instantiating file-backed mappings. > > > > > but I no longer care about DAX, and there's no reason to > > > let DAX hold us back from removing page->index. > > > > Question is whether to move ahead with this now and have Alistair > > rebase, or push ahead with getting Alistair's series into -next? I am > > hoping that Alistair's series can move ahead this cycle, but still > > catching up on the latest after the holiday break. > > The rebase probably isn't that hard, but if we push ahead with my series it's > largely unnecessary as it moves this over to the folio anyway. I've just posted > a respin on top of next-20241216 - > https://lore.kernel.org/linux-mm/cover.425da7c4e76c2749d0ad1734f972b06114e02d52.1736221254.git-series.apopple@nvidia.com/ Looking at what's in linux-next today, there's no changes to dax_set_mapping(), so this patch still applies cleanly (patch 2/2 is obviated). Can you look at this patch (1/2) again and apply it if it seems good?
Matthew Wilcox (Oracle) wrote: > This looks like a complete mess (why are we setting page->index at page > fault time?), but I no longer care about DAX, and there's no reason to > let DAX hold us back from removing page->index. This is a safe conversion for the same reason that Alistair's conversion to vmf_insert_folio_* is safe, folio metadata is always initialized for device-dax. Reviewed-by: Dan Williams <dan.j.williams@intel.com> Andrew, can you pick this one up at the end of Alistair's series?
On Wed, 8 Jan 2025 10:24:00 +1100 Alistair Popple <apopple@nvidia.com> wrote: > > Question is whether to move ahead with this now and have Alistair > > rebase, or push ahead with getting Alistair's series into -next? I am > > hoping that Alistair's series can move ahead this cycle, but still > > catching up on the latest after the holiday break. > > The rebase probably isn't that hard, but if we push ahead with my series it's > largely unnecessary as it moves this over to the folio anyway. I've just posted > a respin on top of next-20241216 - > https://lore.kernel.org/linux-mm/cover.425da7c4e76c2749d0ad1734f972b06114e02d52.1736221254.git-series.apopple@nvidia.com/ I'll drop your v7 series from mm-unstable and shall add these two patches from Matthew.
diff --git a/drivers/dax/device.c b/drivers/dax/device.c index 6d74e62bbee0..bc871a34b9cd 100644 --- a/drivers/dax/device.c +++ b/drivers/dax/device.c @@ -89,14 +89,13 @@ static void dax_set_mapping(struct vm_fault *vmf, pfn_t pfn, ALIGN_DOWN(vmf->address, fault_size)); for (i = 0; i < nr_pages; i++) { - struct page *page = pfn_to_page(pfn_t_to_pfn(pfn) + i); + struct folio *folio = pfn_folio(pfn_t_to_pfn(pfn) + i); - page = compound_head(page); - if (page->mapping) + if (folio->mapping) continue; - page->mapping = filp->f_mapping; - page->index = pgoff + i; + folio->mapping = filp->f_mapping; + folio->index = pgoff + i; } }
This looks like a complete mess (why are we setting page->index at page fault time?), but I no longer care about DAX, and there's no reason to let DAX hold us back from removing page->index. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- drivers/dax/device.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)