[PATCH v2 1/3] arm64: mm: print out correct page table entries

Yury Norov ynorov at caviumnetworks.com
Thu Jun 15 03:12:30 PDT 2017


On Thu, Jun 15, 2017 at 11:00:51AM +0100, Will Deacon wrote:
> On Fri, Jun 09, 2017 at 11:22:36PM +0300, Yury Norov wrote:
> > On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > > index c3f2b1048f83..a9dfb37c87a2 100644
> > > --- a/arch/arm64/mm/fault.c
> > > +++ b/arch/arm64/mm/fault.c
> > > @@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
> > >  #endif
> > >  
> > >  /*
> > > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > > + * Dump out the page tables associated with 'addr' in the currently active mm.
> > >   */
> > > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > > +void show_pte(unsigned long addr)
> > >  {
> > > +	struct mm_struct *mm;
> > >  	pgd_t *pgd;
> > >  
> > > -	if (!mm)
> > > +	if (addr < TASK_SIZE) {
> > > +		/* TTBR0 */
> > > +		mm = current->active_mm;
> > > +	} else if (addr >= VA_START) {
> > > +		/* TTBR1 */
> > >  		mm = &init_mm;
> > > +	} else {
> > > +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> > > +			 addr);
> > > +		return;
> > > +	}
> > >  
> > >  	pr_alert("pgd = %p\n", mm->pgd);
> > >  	pgd = pgd_offset(mm, addr);
> > 
> > Is there any reason to change the prototype of the function?
> > You say nothing about it in patch description. The show_pte()
> > is implemented in several arches, and everywhere it takes mm_struct
> > as 1st argument. For me, if you don't need to change the prototype,
> > you'd better leave things as is. The patch will get much shorter
> > and expressive with it.
> > 
> > If you really need to change the prototype, it would be better to do
> > it in separated patch and give clear explanations - what for?
> 
> The mm is unused and this isn't a core interface. In fact, it's only
> available on architectures that started off by copying from arch/arm/.
> 
> We can always add the mm back if we need it in future.

OK, understood that. But I still think it should be done in separeted
patch, and it would be also good to revise sh and unicore32. It seems
that sh may be easily switched to the proposed interface, and the
unicore32 most probably too.

Yury



More information about the linux-arm-kernel mailing list