Re: [PATCH v2 2/3] mm: Introduce page_shift()

[Ira Weiny <ira.weiny@intel.com>]

On Wed, Jul 24, 2019 at 05:30:55PM -0700, Andrew Morton wrote:
> On Wed, 24 Jul 2019 18:40:25 +0800 kbuild test robot <lkp@intel.com> wrote:
> 
> > Thank you for the patch! Yet something to improve:
> > 
> > [auto build test ERROR on linus/master]
> > [cannot apply to v5.3-rc1 next-20190724]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > 
> > url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox/Make-working-with-compound-pages-easier/20190722-030555
> > config: powerpc64-allyesconfig (attached as .config)
> > compiler: powerpc64-linux-gcc (GCC) 7.4.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.4.0 make.cross ARCH=powerpc64 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > 
> > Note: the linux-review/Matthew-Wilcox/Make-working-with-compound-pages-easier/20190722-030555 HEAD e1bb8b04ba8cf861b2610b0ae646ee49cb069568 builds fine.
> >       It only hurts bisectibility.
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >    drivers/vfio/vfio_iommu_spapr_tce.c: In function 'tce_page_is_contained':
> > >> drivers/vfio/vfio_iommu_spapr_tce.c:193:9: error: called object 'page_shift' is not a function or function pointer
> >      return page_shift(compound_head(page)) >= page_shift;
> >             ^~~~~~~~~~
> >    drivers/vfio/vfio_iommu_spapr_tce.c:179:16: note: declared here
> >       unsigned int page_shift)
> >                    ^~~~~~~~~~
> 
> This?

Looks reasonable to me.  But checking around it does seem like "page_shift" is
used as a parameter or variable in quite a few other places.

Is this something to be concerned with?

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c~mm-introduce-page_shift-fix
> +++ a/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -176,13 +176,13 @@ put_exit:
>  }
>  
>  static bool tce_page_is_contained(struct mm_struct *mm, unsigned long hpa,
> -		unsigned int page_shift)
> +		unsigned int it_page_shift)
>  {
>  	struct page *page;
>  	unsigned long size = 0;
>  
> -	if (mm_iommu_is_devmem(mm, hpa, page_shift, &size))
> -		return size == (1UL << page_shift);
> +	if (mm_iommu_is_devmem(mm, hpa, it_page_shift, &size))
> +		return size == (1UL << it_page_shift);
>  
>  	page = pfn_to_page(hpa >> PAGE_SHIFT);
>  	/*
> @@ -190,7 +190,7 @@ static bool tce_page_is_contained(struct
>  	 * a page we just found. Otherwise the hardware can get access to
>  	 * a bigger memory chunk that it should.
>  	 */
> -	return page_shift(compound_head(page)) >= page_shift;
> +	return page_shift(compound_head(page)) >= it_page_shift;
>  }
>  
>  static inline bool tce_groups_attached(struct tce_container *container)
> _
>