Subject: Re: [PATCH v5 1/4] powerpc/mm: refactor radix physical page mapping

From: Balbir Singh <[email-masked]>
Date: Wed, 18 Jan 2017 06:44:05 +0530

On Tue, Jan 17, 2017 at 12:34:56PM -0600, Reza Arbab wrote:
> Thanks for your review!
> 
> On Tue, Jan 17, 2017 at 12:16:35PM +0530, Balbir Singh wrote:
> > On Mon, Jan 16, 2017 at 01:07:43PM -0600, Reza Arbab wrote:
> > > --- a/arch/powerpc/mm/pgtable-radix.c
> > > +++ b/arch/powerpc/mm/pgtable-radix.c
> > > @@ -107,54 +107,66 @@ int radix__map_kernel_page(unsigned long ea, unsigned long pa,
> > >  	return 0;
> > >  }
> > > 
> > > +static inline void __meminit print_mapping(unsigned long start,
> > > +					   unsigned long end,
> > > +					   unsigned long size)
> > > +{
> > > +	if (end <= start)
> > > +		return;
> > 
> > Should we pr_err for start > end?
> 
> I think that would be overkill. The way this little inline is called, start
> > end is not possible. The real point is not to print anything if start ==
> end. Using <= just seemed better in context.
>

Agreed

<snip>
 
> > 
> > Should we try a lower page size if map_kernel_page fails for this mapping_size?
> 
> The only way map_kernel_page can fail is -ENOMEM. If that's the case,
> there's no way we're going to be able to map this range at all. Better to
> fail fast here, I would think.
>

I think I am OK with this implementation for now. 

Balbir Singh.