Re: [PATCH 2/2] mm/gup: fix a misnamed "write" argument: should be "flags"

From: Aneesh Kumar K.V
Date: Mon Oct 14 2019 - 10:44:41 EST


On 10/14/19 7:22 PM, Kirill A. Shutemov wrote:
On Sun, Oct 13, 2019 at 11:43:10PM -0700, John Hubbard wrote:
On 10/13/19 11:12 PM, kbuild test robot wrote:
Hi John,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc3 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/John-Hubbard/gup-c-gup_benchmark-c-trivial-fixes-before-the-storm/20191014-114158
config: powerpc-defconfig (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=powerpc

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@xxxxxxxxx>

All errors (new ones prefixed by >>):

mm/gup.c: In function 'gup_hugepte':
mm/gup.c:1990:33: error: 'write' undeclared (first use in this function); did you mean 'writeq'?
if (!pte_access_permitted(pte, write))
^~~~~
writeq
mm/gup.c:1990:33: note: each undeclared identifier is reported only once for each function it appears in


OK, so this shows that my cross-compiler test scripts are faulty lately,
sorry I missed this.

But more importantly, the above missed case is an example of when "write" really
means "write", as opposed to meaning flags.

Please put this patch on hold or drop it, until we hear from the authors as to how
they would like to resolve this. I suspect it will end up as something like:

bool write = (flags & FOLL_WRITE);

...perhaps?

Just use

if (!pte_access_permitted(pte, flags & FOLL_WRITE))

as we have in gup_pte_range().

And add:

Fixes: cbd34da7dc9a ("mm: move the powerpc hugepd code to mm/gup.c")


b798bec4741bdd80224214fdd004c8e52698e42 isn't this the commit that need to be mentioned in the Fixes: tag?

-aneesh