Re: [PATCH v3 1/8] mm/dax: Dump start address in fault handler

[Peter Xu <peterx@redhat.com>]

On Wed, Jul 31, 2024 at 02:04:38PM +0200, David Hildenbrand wrote:
> On 15.07.24 21:21, Peter Xu wrote:
> > Currently the dax fault handler dumps the vma range when dynamic debugging
> > enabled.  That's mostly not useful.  Dump the (aligned) address instead
> > with the order info.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   drivers/dax/device.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> > index eb61598247a9..714174844ca5 100644
> > --- a/drivers/dax/device.c
> > +++ b/drivers/dax/device.c
> > @@ -235,9 +235,9 @@ static vm_fault_t dev_dax_huge_fault(struct vm_fault *vmf, unsigned int order)
> >   	int id;
> >   	struct dev_dax *dev_dax = filp->private_data;
> > -	dev_dbg(&dev_dax->dev, "%s: %s (%#lx - %#lx) order:%d\n", current->comm,
> > -			(vmf->flags & FAULT_FLAG_WRITE) ? "write" : "read",
> > -			vmf->vma->vm_start, vmf->vma->vm_end, order);
> > +	dev_dbg(&dev_dax->dev, "%s: op=%s addr=%#lx order=%d\n", current->comm,
> > +		(vmf->flags & FAULT_FLAG_WRITE) ? "write" : "read",
> > +		vmf->address & ~((1UL << (order + PAGE_SHIFT)) - 1), order);
> >   	id = dax_read_lock();
> >   	if (order == 0)
> 
> Agreed, the address of the fault is better. Just wondering, would the
> unmasked address be even better? Using the order we can figure out the
> to-be-aligned address.

From my very limited dax experience on monitoring these faults and making
sure huge mappings popped up correctly.. it's so far easier when we see
address properly aligned with order info.  But let me know if you still
prefer that, I'm fine with making that calculation simpler.

> 
> Acked-by: David Hildenbrand <david@redhat.com>

Thanks for taking a look!

-- 
Peter Xu