| Message ID | 20220419045045.1664996-4-ruansy.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fsdax: introduce fs query to support reflink | expand |
On Tue, Apr 19, 2022 at 12:50:41PM +0800, Shiyang Ruan wrote: > When memory-failure occurs, we call this function which is implemented > by each kind of devices. For the fsdax case, pmem device driver > implements it. Pmem device driver will find out the filesystem in which > the corrupted page located in. > > With dax_holder notify support, we are able to notify the memory failure > from pmem driver to upper layers. If there is something not support in > the notify routine, memory_failure will fall back to the generic hanlder. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Dan Williams <dan.j.williams@intel.com> Looks good to me now that we've ironed out the earlier unit questions, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > drivers/nvdimm/pmem.c | 17 +++++++++++++++++ > include/linux/memremap.h | 12 ++++++++++++ > mm/memory-failure.c | 14 ++++++++++++++ > 3 files changed, 43 insertions(+) > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 58d95242a836..bd502957cfdf 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -366,6 +366,21 @@ static void pmem_release_disk(void *__pmem) > blk_cleanup_disk(pmem->disk); > } > > +static int pmem_pagemap_memory_failure(struct dev_pagemap *pgmap, > + unsigned long pfn, unsigned long nr_pages, int mf_flags) > +{ > + struct pmem_device *pmem = > + container_of(pgmap, struct pmem_device, pgmap); > + u64 offset = PFN_PHYS(pfn) - pmem->phys_addr - pmem->data_offset; > + u64 len = nr_pages << PAGE_SHIFT; > + > + return dax_holder_notify_failure(pmem->dax_dev, offset, len, mf_flags); > +} > + > +static const struct dev_pagemap_ops fsdax_pagemap_ops = { > + .memory_failure = pmem_pagemap_memory_failure, > +}; > + > static int pmem_attach_disk(struct device *dev, > struct nd_namespace_common *ndns) > { > @@ -427,6 +442,7 @@ static int pmem_attach_disk(struct device *dev, > pmem->pfn_flags = PFN_DEV; > if (is_nd_pfn(dev)) { > pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; > + pmem->pgmap.ops = &fsdax_pagemap_ops; > addr = devm_memremap_pages(dev, &pmem->pgmap); > pfn_sb = nd_pfn->pfn_sb; > pmem->data_offset = le64_to_cpu(pfn_sb->dataoff); > @@ -440,6 +456,7 @@ static int pmem_attach_disk(struct device *dev, > pmem->pgmap.range.end = res->end; > pmem->pgmap.nr_range = 1; > pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; > + pmem->pgmap.ops = &fsdax_pagemap_ops; > addr = devm_memremap_pages(dev, &pmem->pgmap); > pmem->pfn_flags |= PFN_MAP; > bb_range = pmem->pgmap.range; > diff --git a/include/linux/memremap.h b/include/linux/memremap.h > index ad6062d736cd..bcfb6bf4ce5a 100644 > --- a/include/linux/memremap.h > +++ b/include/linux/memremap.h > @@ -79,6 +79,18 @@ struct dev_pagemap_ops { > * the page back to a CPU accessible page. > */ > vm_fault_t (*migrate_to_ram)(struct vm_fault *vmf); > + > + /* > + * Handle the memory failure happens on a range of pfns. Notify the > + * processes who are using these pfns, and try to recover the data on > + * them if necessary. The mf_flags is finally passed to the recover > + * function through the whole notify routine. > + * > + * When this is not implemented, or it returns -EOPNOTSUPP, the caller > + * will fall back to a common handler called mf_generic_kill_procs(). > + */ > + int (*memory_failure)(struct dev_pagemap *pgmap, unsigned long pfn, > + unsigned long nr_pages, int mf_flags); > }; > > #define PGMAP_ALTMAP_VALID (1 << 0) > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 7c8c047bfdc8..a40e79e634a4 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1741,6 +1741,20 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, > if (!pgmap_pfn_valid(pgmap, pfn)) > goto out; > > + /* > + * Call driver's implementation to handle the memory failure, otherwise > + * fall back to generic handler. > + */ > + if (pgmap->ops->memory_failure) { > + rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); > + /* > + * Fall back to generic handler too if operation is not > + * supported inside the driver/device/filesystem. > + */ > + if (rc != -EOPNOTSUPP) > + goto out; > + } > + > rc = mf_generic_kill_procs(pfn, flags, pgmap); > out: > /* drop pgmap ref acquired in caller */ > -- > 2.35.1 > > >
On Tue, Apr 19, 2022 at 12:50:41PM +0800, Shiyang Ruan wrote: > When memory-failure occurs, we call this function which is implemented > by each kind of devices. For the fsdax case, pmem device driver > implements it. Pmem device driver will find out the filesystem in which > the corrupted page located in. > > With dax_holder notify support, we are able to notify the memory failure > from pmem driver to upper layers. If there is something not support in > the notify routine, memory_failure will fall back to the generic hanlder. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Dan Williams <dan.j.williams@intel.com> Looks good to me, thank you. Reviewed-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
On 2022/4/19 12:50, Shiyang Ruan wrote: > When memory-failure occurs, we call this function which is implemented > by each kind of devices. For the fsdax case, pmem device driver > implements it. Pmem device driver will find out the filesystem in which > the corrupted page located in. > > With dax_holder notify support, we are able to notify the memory failure > from pmem driver to upper layers. If there is something not support in > the notify routine, memory_failure will fall back to the generic hanlder. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/nvdimm/pmem.c | 17 +++++++++++++++++ > include/linux/memremap.h | 12 ++++++++++++ > mm/memory-failure.c | 14 ++++++++++++++ > 3 files changed, 43 insertions(+) > > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 58d95242a836..bd502957cfdf 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -366,6 +366,21 @@ static void pmem_release_disk(void *__pmem) > blk_cleanup_disk(pmem->disk); > } > > +static int pmem_pagemap_memory_failure(struct dev_pagemap *pgmap, > + unsigned long pfn, unsigned long nr_pages, int mf_flags) > +{ > + struct pmem_device *pmem = > + container_of(pgmap, struct pmem_device, pgmap); > + u64 offset = PFN_PHYS(pfn) - pmem->phys_addr - pmem->data_offset; > + u64 len = nr_pages << PAGE_SHIFT; > + > + return dax_holder_notify_failure(pmem->dax_dev, offset, len, mf_flags); > +} > + > +static const struct dev_pagemap_ops fsdax_pagemap_ops = { > + .memory_failure = pmem_pagemap_memory_failure, > +}; > + > static int pmem_attach_disk(struct device *dev, > struct nd_namespace_common *ndns) > { > @@ -427,6 +442,7 @@ static int pmem_attach_disk(struct device *dev, > pmem->pfn_flags = PFN_DEV; > if (is_nd_pfn(dev)) { > pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; > + pmem->pgmap.ops = &fsdax_pagemap_ops; > addr = devm_memremap_pages(dev, &pmem->pgmap); > pfn_sb = nd_pfn->pfn_sb; > pmem->data_offset = le64_to_cpu(pfn_sb->dataoff); > @@ -440,6 +456,7 @@ static int pmem_attach_disk(struct device *dev, > pmem->pgmap.range.end = res->end; > pmem->pgmap.nr_range = 1; > pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; > + pmem->pgmap.ops = &fsdax_pagemap_ops; > addr = devm_memremap_pages(dev, &pmem->pgmap); > pmem->pfn_flags |= PFN_MAP; > bb_range = pmem->pgmap.range; > diff --git a/include/linux/memremap.h b/include/linux/memremap.h > index ad6062d736cd..bcfb6bf4ce5a 100644 > --- a/include/linux/memremap.h > +++ b/include/linux/memremap.h > @@ -79,6 +79,18 @@ struct dev_pagemap_ops { > * the page back to a CPU accessible page. > */ > vm_fault_t (*migrate_to_ram)(struct vm_fault *vmf); > + > + /* > + * Handle the memory failure happens on a range of pfns. Notify the > + * processes who are using these pfns, and try to recover the data on > + * them if necessary. The mf_flags is finally passed to the recover > + * function through the whole notify routine. > + * > + * When this is not implemented, or it returns -EOPNOTSUPP, the caller > + * will fall back to a common handler called mf_generic_kill_procs(). > + */ > + int (*memory_failure)(struct dev_pagemap *pgmap, unsigned long pfn, > + unsigned long nr_pages, int mf_flags); > }; > > #define PGMAP_ALTMAP_VALID (1 << 0) > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 7c8c047bfdc8..a40e79e634a4 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1741,6 +1741,20 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, > if (!pgmap_pfn_valid(pgmap, pfn)) > goto out; > > + /* > + * Call driver's implementation to handle the memory failure, otherwise > + * fall back to generic handler. > + */ > + if (pgmap->ops->memory_failure) { > + rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); > + /* > + * Fall back to generic handler too if operation is not > + * supported inside the driver/device/filesystem. > + */ > + if (rc != -EOPNOTSUPP) > + goto out; > + } > + Thanks for your patch. There are two questions: 1.Is dax_lock_page + dax_unlock_page pair needed here? 2.hwpoison_filter and SetPageHWPoison will be handled by the callback or they're just ignored deliberately? Thanks! > rc = mf_generic_kill_procs(pfn, flags, pgmap); > out: > /* drop pgmap ref acquired in caller */ >
在 2022/4/21 16:24, Miaohe Lin 写道: > On 2022/4/19 12:50, Shiyang Ruan wrote: >> When memory-failure occurs, we call this function which is implemented >> by each kind of devices. For the fsdax case, pmem device driver >> implements it. Pmem device driver will find out the filesystem in which >> the corrupted page located in. >> >> With dax_holder notify support, we are able to notify the memory failure >> from pmem driver to upper layers. If there is something not support in >> the notify routine, memory_failure will fall back to the generic hanlder. >> >> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Reviewed-by: Dan Williams <dan.j.williams@intel.com> >> --- >> drivers/nvdimm/pmem.c | 17 +++++++++++++++++ >> include/linux/memremap.h | 12 ++++++++++++ >> mm/memory-failure.c | 14 ++++++++++++++ >> 3 files changed, 43 insertions(+) >> >> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c >> index 58d95242a836..bd502957cfdf 100644 >> --- a/drivers/nvdimm/pmem.c >> +++ b/drivers/nvdimm/pmem.c >> @@ -366,6 +366,21 @@ static void pmem_release_disk(void *__pmem) >> blk_cleanup_disk(pmem->disk); >> } >> >> +static int pmem_pagemap_memory_failure(struct dev_pagemap *pgmap, >> + unsigned long pfn, unsigned long nr_pages, int mf_flags) >> +{ >> + struct pmem_device *pmem = >> + container_of(pgmap, struct pmem_device, pgmap); >> + u64 offset = PFN_PHYS(pfn) - pmem->phys_addr - pmem->data_offset; >> + u64 len = nr_pages << PAGE_SHIFT; >> + >> + return dax_holder_notify_failure(pmem->dax_dev, offset, len, mf_flags); >> +} >> + >> +static const struct dev_pagemap_ops fsdax_pagemap_ops = { >> + .memory_failure = pmem_pagemap_memory_failure, >> +}; >> + >> static int pmem_attach_disk(struct device *dev, >> struct nd_namespace_common *ndns) >> { >> @@ -427,6 +442,7 @@ static int pmem_attach_disk(struct device *dev, >> pmem->pfn_flags = PFN_DEV; >> if (is_nd_pfn(dev)) { >> pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; >> + pmem->pgmap.ops = &fsdax_pagemap_ops; >> addr = devm_memremap_pages(dev, &pmem->pgmap); >> pfn_sb = nd_pfn->pfn_sb; >> pmem->data_offset = le64_to_cpu(pfn_sb->dataoff); >> @@ -440,6 +456,7 @@ static int pmem_attach_disk(struct device *dev, >> pmem->pgmap.range.end = res->end; >> pmem->pgmap.nr_range = 1; >> pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; >> + pmem->pgmap.ops = &fsdax_pagemap_ops; >> addr = devm_memremap_pages(dev, &pmem->pgmap); >> pmem->pfn_flags |= PFN_MAP; >> bb_range = pmem->pgmap.range; >> diff --git a/include/linux/memremap.h b/include/linux/memremap.h >> index ad6062d736cd..bcfb6bf4ce5a 100644 >> --- a/include/linux/memremap.h >> +++ b/include/linux/memremap.h >> @@ -79,6 +79,18 @@ struct dev_pagemap_ops { >> * the page back to a CPU accessible page. >> */ >> vm_fault_t (*migrate_to_ram)(struct vm_fault *vmf); >> + >> + /* >> + * Handle the memory failure happens on a range of pfns. Notify the >> + * processes who are using these pfns, and try to recover the data on >> + * them if necessary. The mf_flags is finally passed to the recover >> + * function through the whole notify routine. >> + * >> + * When this is not implemented, or it returns -EOPNOTSUPP, the caller >> + * will fall back to a common handler called mf_generic_kill_procs(). >> + */ >> + int (*memory_failure)(struct dev_pagemap *pgmap, unsigned long pfn, >> + unsigned long nr_pages, int mf_flags); >> }; >> >> #define PGMAP_ALTMAP_VALID (1 << 0) >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index 7c8c047bfdc8..a40e79e634a4 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -1741,6 +1741,20 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, >> if (!pgmap_pfn_valid(pgmap, pfn)) >> goto out; >> >> + /* >> + * Call driver's implementation to handle the memory failure, otherwise >> + * fall back to generic handler. >> + */ >> + if (pgmap->ops->memory_failure) { >> + rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); >> + /* >> + * Fall back to generic handler too if operation is not >> + * supported inside the driver/device/filesystem. >> + */ >> + if (rc != -EOPNOTSUPP) >> + goto out; >> + } >> + > > Thanks for your patch. There are two questions: > > 1.Is dax_lock_page + dax_unlock_page pair needed here? They are moved into mf_generic_kill_procs() in Patch2. Callback will implement its own dax lock/unlock method. For example, for mf_dax_kill_procs() in Patch4, we implemented dax_lock_mapping_entry()/dax_unlock_mapping_entry() for it. > 2.hwpoison_filter and SetPageHWPoison will be handled by the callback or they're just ignored deliberately? SetPageHWPoison() will be handled by callback or by mf_generic_kill_procs(). hwpoison_filter() is moved into mf_generic_kill_procs() too. The callback will make sure the page is correct, so it is ignored. -- Thanks, Ruan. > > Thanks! > >> rc = mf_generic_kill_procs(pfn, flags, pgmap); >> out: >> /* drop pgmap ref acquired in caller */ >> >
On 2022/4/22 15:06, Shiyang Ruan wrote: > > ... >> >> Thanks for your patch. There are two questions: >> >> 1.Is dax_lock_page + dax_unlock_page pair needed here? > > They are moved into mf_generic_kill_procs() in Patch2. Callback will implement its own dax lock/unlock method. For example, for mf_dax_kill_procs() in Patch4, we implemented dax_lock_mapping_entry()/dax_unlock_mapping_entry() for it. > >> 2.hwpoison_filter and SetPageHWPoison will be handled by the callback or they're just ignored deliberately? > > SetPageHWPoison() will be handled by callback or by mf_generic_kill_procs(). > > hwpoison_filter() is moved into mf_generic_kill_procs() too. The callback will make sure the page is correct, so it is ignored. I see this when I read the other patches. Many thanks for clarifying! > > > -- > Thanks, > Ruan. > >> >> Thanks! >> >>> rc = mf_generic_kill_procs(pfn, flags, pgmap); >>> out: >>> /* drop pgmap ref acquired in caller */ >>> >> > > > .
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 58d95242a836..bd502957cfdf 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -366,6 +366,21 @@ static void pmem_release_disk(void *__pmem) blk_cleanup_disk(pmem->disk); } +static int pmem_pagemap_memory_failure(struct dev_pagemap *pgmap, + unsigned long pfn, unsigned long nr_pages, int mf_flags) +{ + struct pmem_device *pmem = + container_of(pgmap, struct pmem_device, pgmap); + u64 offset = PFN_PHYS(pfn) - pmem->phys_addr - pmem->data_offset; + u64 len = nr_pages << PAGE_SHIFT; + + return dax_holder_notify_failure(pmem->dax_dev, offset, len, mf_flags); +} + +static const struct dev_pagemap_ops fsdax_pagemap_ops = { + .memory_failure = pmem_pagemap_memory_failure, +}; + static int pmem_attach_disk(struct device *dev, struct nd_namespace_common *ndns) { @@ -427,6 +442,7 @@ static int pmem_attach_disk(struct device *dev, pmem->pfn_flags = PFN_DEV; if (is_nd_pfn(dev)) { pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; + pmem->pgmap.ops = &fsdax_pagemap_ops; addr = devm_memremap_pages(dev, &pmem->pgmap); pfn_sb = nd_pfn->pfn_sb; pmem->data_offset = le64_to_cpu(pfn_sb->dataoff); @@ -440,6 +456,7 @@ static int pmem_attach_disk(struct device *dev, pmem->pgmap.range.end = res->end; pmem->pgmap.nr_range = 1; pmem->pgmap.type = MEMORY_DEVICE_FS_DAX; + pmem->pgmap.ops = &fsdax_pagemap_ops; addr = devm_memremap_pages(dev, &pmem->pgmap); pmem->pfn_flags |= PFN_MAP; bb_range = pmem->pgmap.range; diff --git a/include/linux/memremap.h b/include/linux/memremap.h index ad6062d736cd..bcfb6bf4ce5a 100644 --- a/include/linux/memremap.h +++ b/include/linux/memremap.h @@ -79,6 +79,18 @@ struct dev_pagemap_ops { * the page back to a CPU accessible page. */ vm_fault_t (*migrate_to_ram)(struct vm_fault *vmf); + + /* + * Handle the memory failure happens on a range of pfns. Notify the + * processes who are using these pfns, and try to recover the data on + * them if necessary. The mf_flags is finally passed to the recover + * function through the whole notify routine. + * + * When this is not implemented, or it returns -EOPNOTSUPP, the caller + * will fall back to a common handler called mf_generic_kill_procs(). + */ + int (*memory_failure)(struct dev_pagemap *pgmap, unsigned long pfn, + unsigned long nr_pages, int mf_flags); }; #define PGMAP_ALTMAP_VALID (1 << 0) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 7c8c047bfdc8..a40e79e634a4 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1741,6 +1741,20 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, if (!pgmap_pfn_valid(pgmap, pfn)) goto out; + /* + * Call driver's implementation to handle the memory failure, otherwise + * fall back to generic handler. + */ + if (pgmap->ops->memory_failure) { + rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); + /* + * Fall back to generic handler too if operation is not + * supported inside the driver/device/filesystem. + */ + if (rc != -EOPNOTSUPP) + goto out; + } + rc = mf_generic_kill_procs(pfn, flags, pgmap); out: /* drop pgmap ref acquired in caller */