Re: [PATCH V4 2/9] dax/fsdev: fix multi-range offset in memory_failure handler

[John Groves]

On 26/06/08 06:56PM, Richard Cheng wrote:
> On Sun, Jun 07, 2026 at 07:33:19PM +0800, John Groves wrote:
> > From: John Groves <John@xxxxxxxxxx>
> > 
> > Fix memory_failure offset calculation for multi-range devices. The old code
> > subtracted ranges[0].range.start from the faulting PFN's physical address,
> > which produces an incorrect (inflated) logical offset when the PFN falls in
> > ranges[1] or beyond due to physical gaps between ranges. Add
> > fsdev_pfn_to_offset() to walk the range list and compute the correct
> > device-linear byte offset.
> > 
> > Fixes: d5406bd458b0a ("dax: add fsdev.c driver for fs-dax on character dax")
> > 
> > Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> > Reviewed-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> > Signed-off-by: John Groves <john@xxxxxxxxxx>
> > ---
> >  drivers/dax/fsdev.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> > index 188b2526bee45..f315533b299e9 100644
> > --- a/drivers/dax/fsdev.c
> > +++ b/drivers/dax/fsdev.c
> > @@ -135,11 +135,26 @@ static void fsdev_clear_ops(void *data)
> >   * The core mm code in free_zone_device_folio() handles the wake_up_var()
> >   * directly for this memory type.
> >   */
> > +static u64 fsdev_pfn_to_offset(struct dev_dax *dev_dax, unsigned long pfn)
> > +{
> > +	phys_addr_t phys = PFN_PHYS(pfn);
> > +	u64 offset = 0;
> > +
> > +	for (int i = 0; i < dev_dax->nr_range; i++) {
> > +		struct range *range = &dev_dax->ranges[i].range;
> > +
> 
> IMHO, this walks dev_dax->ranges[] locklessly from the memory_failure callback.
> mapping_store() can krealloc() that array via alloc_dev_dax_range() without
> checking dev->driver, so a sysfs mapping write concurrent with a HW poison event
> can move/free it under this walk.
> 
> We have pgmap->ranges[], the imuutable copy populated at probe and never mutated
> afterwards, right here in the callback, and its accumulated range_len() is exactly
> the device-linear offset.
> Maybe walking that instread closes the race.
> 
> What do you think?

Good idea. Revising to use the pgmap->ranges instead. Thanks Richard!

Regards,
John

<snip>