From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>,
Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
kbuild test robot <lkp@intel.com>, <kbuild-all@lists.01.org>,
Andrew Morton <akpm@linux-foundation.org>,
Christoph Hellwig <hch@infradead.org>,
"Keith Busch" <keith.busch@intel.com>,
LKML <linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 2/2] mm/gup: fix a misnamed "write" argument: should be "flags"
Date: Mon, 14 Oct 2019 11:39:33 -0700 [thread overview]
Message-ID: <e287e070-8d9b-5455-7a70-4c6cf10b5633@nvidia.com> (raw)
In-Reply-To: <20191014164513.GA6839@iweiny-DESK2.sc.intel.com>
On 10/14/19 9:45 AM, Ira Weiny wrote:
> On Mon, Oct 14, 2019 at 08:14:04PM +0530, Aneesh Kumar K.V wrote:
>> 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@intel.com>
>>>>>
>>>>> 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?
>
> Yes, and while we are at it the type should probably be changed to unsigned
> int.
>
OK, I'm posting a v2 with all the above, thanks for these reviews!
thanks,
John Hubbard
NVIDIA
prev parent reply other threads:[~2019-10-14 18:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-13 22:11 [PATCH 0/2] gup.c, gup_benchmark.c trivial fixes before the storm John Hubbard
2019-10-13 22:11 ` [PATCH 1/2] mm/gup_benchmark: add a missing "w" to getopt string John Hubbard
2019-10-14 13:36 ` Kirill A. Shutemov
2019-10-13 22:11 ` [PATCH 2/2] mm/gup: fix a misnamed "write" argument: should be "flags" John Hubbard
2019-10-14 6:12 ` kbuild test robot
2019-10-14 6:43 ` John Hubbard
2019-10-14 13:52 ` Kirill A. Shutemov
2019-10-14 14:44 ` Aneesh Kumar K.V
2019-10-14 14:51 ` Kirill A. Shutemov
2019-10-14 16:45 ` Ira Weiny
2019-10-14 18:39 ` John Hubbard [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e287e070-8d9b-5455-7a70-4c6cf10b5633@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=akpm@linux-foundation.org \
--cc=aneesh.kumar@linux.ibm.com \
--cc=hch@infradead.org \
--cc=hch@lst.de \
--cc=ira.weiny@intel.com \
--cc=kbuild-all@lists.01.org \
--cc=keith.busch@intel.com \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).