| Message ID | 20210329193828.179993-1-shy828301@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: gup: remove FOLL_SPLIT | expand |
Hi Yang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on hnaz-linux-mm/master]
url: https://github.com/0day-ci/linux/commits/Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042
base: https://github.com/hnaz/linux-mm master
config: s390-randconfig-r032-20210330 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c8563a636718f98af86a3965d94e25b8f2cf2354
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042
git checkout c8563a636718f98af86a3965d94e25b8f2cf2354
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/s390/mm/gmap.c: In function 'thp_split_mm':
>> arch/s390/mm/gmap.c:2498:27: error: 'FOLL_SPLIT' undeclared (first use in this function); did you mean 'FOLL_PIN'?
2498 | follow_page(vma, addr, FOLL_SPLIT);
| ^~~~~~~~~~
| FOLL_PIN
arch/s390/mm/gmap.c:2498:27: note: each undeclared identifier is reported only once for each function it appears in
vim +2498 arch/s390/mm/gmap.c
0959e168678d2d Janosch Frank 2018-07-17 2487
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2488 static inline void thp_split_mm(struct mm_struct *mm)
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2489 {
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2490 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2491 struct vm_area_struct *vma;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2492 unsigned long addr;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2493
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2494 for (vma = mm->mmap; vma != NULL; vma = vma->vm_next) {
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2495 for (addr = vma->vm_start;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2496 addr < vma->vm_end;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2497 addr += PAGE_SIZE)
1e133ab296f3ff Martin Schwidefsky 2016-03-08 @2498 follow_page(vma, addr, FOLL_SPLIT);
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2499 vma->vm_flags &= ~VM_HUGEPAGE;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2500 vma->vm_flags |= VM_NOHUGEPAGE;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2501 }
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2502 mm->def_flags |= VM_NOHUGEPAGE;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2503 #endif
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2504 }
1e133ab296f3ff Martin Schwidefsky 2016-03-08 2505
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 3/29/21 12:38 PM, Yang Shi wrote: > Since commit 5a52c9df62b4 ("uprobe: use FOLL_SPLIT_PMD instead of FOLL_SPLIT") > and commit ba925fa35057 ("s390/gmap: improve THP splitting") FOLL_SPLIT > has not been used anymore. Remove the dead code. > > Signed-off-by: Yang Shi <shy828301@gmail.com> > --- > include/linux/mm.h | 1 - > mm/gup.c | 28 ++-------------------------- > 2 files changed, 2 insertions(+), 27 deletions(-) > Looks nice. As long as I'm running git grep here, there is one more search hit that should also be fixed up, as part of a "remove FOLL_SPLIT" patch: git grep -nw FOLL_SPLIT Documentation/vm/transhuge.rst:57:follow_page, the FOLL_SPLIT bit can be specified as a parameter to Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
On 3/29/21 11:24 PM, kernel test robot wrote: > Hi Yang, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on hnaz-linux-mm/master] > > url: https://github.com/0day-ci/linux/commits/Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042 > base: https://github.com/hnaz/linux-mm master > config: s390-randconfig-r032-20210330 (attached as .config) > compiler: s390-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/0day-ci/linux/commit/c8563a636718f98af86a3965d94e25b8f2cf2354 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042 > git checkout c8563a636718f98af86a3965d94e25b8f2cf2354 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > arch/s390/mm/gmap.c: In function 'thp_split_mm': >>> arch/s390/mm/gmap.c:2498:27: error: 'FOLL_SPLIT' undeclared (first use in this function); did you mean 'FOLL_PIN'? > 2498 | follow_page(vma, addr, FOLL_SPLIT); > | ^~~~~~~~~~ > | FOLL_PIN > arch/s390/mm/gmap.c:2498:27: note: each undeclared identifier is reported only once for each function it appears in > > > vim +2498 arch/s390/mm/gmap.c There appears to be an imperfection in this 0day testing system, because (just as the patch says), commit ba925fa35057a062ac98c3e8138b013ce4ce351c ("s390/gmap: improve THP splitting"), July 29, 2020, removes the above use of FOLL_SPLIT. And "git grep", just to be sure, shows it is not there in today's linux.git. So I guess the https://github.com/0day-ci/linux repo needs a better way to stay in sync? thanks,
On Tue, Mar 30, 2021 at 12:08 AM John Hubbard <jhubbard@nvidia.com> wrote: > > On 3/29/21 12:38 PM, Yang Shi wrote: > > Since commit 5a52c9df62b4 ("uprobe: use FOLL_SPLIT_PMD instead of FOLL_SPLIT") > > and commit ba925fa35057 ("s390/gmap: improve THP splitting") FOLL_SPLIT > > has not been used anymore. Remove the dead code. > > > > Signed-off-by: Yang Shi <shy828301@gmail.com> > > --- > > include/linux/mm.h | 1 - > > mm/gup.c | 28 ++-------------------------- > > 2 files changed, 2 insertions(+), 27 deletions(-) > > > > Looks nice. > > As long as I'm running git grep here, there is one more search hit that should also > be fixed up, as part of a "remove FOLL_SPLIT" patch: > > git grep -nw FOLL_SPLIT > Documentation/vm/transhuge.rst:57:follow_page, the FOLL_SPLIT bit can be specified as a parameter to > > Reviewed-by: John Hubbard <jhubbard@nvidia.com> Thanks. Removed the reference to FOLL_SPLIT in documentation for v2. > > thanks, > -- > John Hubbard > NVIDIA > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 8ba434287387..3568836841f9 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -2780,7 +2780,6 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address, > > #define FOLL_NOWAIT 0x20 /* if a disk transfer is needed, start the IO > > * and return without waiting upon it */ > > #define FOLL_POPULATE 0x40 /* fault in page */ > > -#define FOLL_SPLIT 0x80 /* don't return transhuge pages, split them */ > > #define FOLL_HWPOISON 0x100 /* check page is hwpoisoned */ > > #define FOLL_NUMA 0x200 /* force NUMA hinting page fault */ > > #define FOLL_MIGRATION 0x400 /* wait for page to replace migration entry */ > > diff --git a/mm/gup.c b/mm/gup.c > > index e40579624f10..f3d45a8f18ae 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -435,18 +435,6 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, > > } > > } > > > > - if (flags & FOLL_SPLIT && PageTransCompound(page)) { > > - get_page(page); > > - pte_unmap_unlock(ptep, ptl); > > - lock_page(page); > > - ret = split_huge_page(page); > > - unlock_page(page); > > - put_page(page); > > - if (ret) > > - return ERR_PTR(ret); > > - goto retry; > > - } > > - > > /* try_grab_page() does nothing unless FOLL_GET or FOLL_PIN is set. */ > > if (unlikely(!try_grab_page(page, flags))) { > > page = ERR_PTR(-ENOMEM); > > @@ -591,7 +579,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, > > spin_unlock(ptl); > > return follow_page_pte(vma, address, pmd, flags, &ctx->pgmap); > > } > > - if (flags & (FOLL_SPLIT | FOLL_SPLIT_PMD)) { > > + if (flags & FOLL_SPLIT_PMD) { > > int ret; > > page = pmd_page(*pmd); > > if (is_huge_zero_page(page)) { > > @@ -600,19 +588,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, > > split_huge_pmd(vma, pmd, address); > > if (pmd_trans_unstable(pmd)) > > ret = -EBUSY; > > - } else if (flags & FOLL_SPLIT) { > > - if (unlikely(!try_get_page(page))) { > > - spin_unlock(ptl); > > - return ERR_PTR(-ENOMEM); > > - } > > - spin_unlock(ptl); > > - lock_page(page); > > - ret = split_huge_page(page); > > - unlock_page(page); > > - put_page(page); > > - if (pmd_none(*pmd)) > > - return no_page_table(vma, flags); > > - } else { /* flags & FOLL_SPLIT_PMD */ > > + } else { > > spin_unlock(ptl); > > split_huge_pmd(vma, pmd, address); > > ret = pte_alloc(mm, pmd) ? -ENOMEM : 0; > > >
Hi John, On 3/30/21 3:08 PM, John Hubbard wrote: > On 3/29/21 11:24 PM, kernel test robot wrote: >> Hi Yang, >> >> Thank you for the patch! Yet something to improve: >> >> [auto build test ERROR on hnaz-linux-mm/master] >> >> url: >> https://github.com/0day-ci/linux/commits/Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042 >> base: https://github.com/hnaz/linux-mm master >> config: s390-randconfig-r032-20210330 (attached as .config) >> compiler: s390-linux-gcc (GCC) 9.3.0 >> reproduce (this is a W=1 build): >> wget >> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross >> -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # >> https://github.com/0day-ci/linux/commit/c8563a636718f98af86a3965d94e25b8f2cf2354 >> git remote add linux-review https://github.com/0day-ci/linux >> git fetch --no-tags linux-review >> Yang-Shi/mm-gup-remove-FOLL_SPLIT/20210330-034042 >> git checkout c8563a636718f98af86a3965d94e25b8f2cf2354 >> # save the attached .config to linux build tree >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 >> make.cross ARCH=s390 >> >> If you fix the issue, kindly add following tag as appropriate >> Reported-by: kernel test robot <lkp@intel.com> >> >> All errors (new ones prefixed by >>): >> >> arch/s390/mm/gmap.c: In function 'thp_split_mm': >>>> arch/s390/mm/gmap.c:2498:27: error: 'FOLL_SPLIT' undeclared (first >>>> use in this function); did you mean 'FOLL_PIN'? >> 2498 | follow_page(vma, addr, FOLL_SPLIT); >> | ^~~~~~~~~~ >> | FOLL_PIN >> arch/s390/mm/gmap.c:2498:27: note: each undeclared identifier is >> reported only once for each function it appears in >> >> >> vim +2498 arch/s390/mm/gmap.c > > There appears to be an imperfection in this 0day testing system, > because (just as the patch > says), commit ba925fa35057a062ac98c3e8138b013ce4ce351c ("s390/gmap: > improve THP splitting"), > July 29, 2020, removes the above use of FOLL_SPLIT. > > And "git grep", just to be sure, shows it is not there in today's > linux.git. So I guess the > https://github.com/0day-ci/linux repo needs a better way to stay in sync? Sorry for the delay, indeed, it's a issue from 0day-CI, we'll update linux-mm in the system. Best Regards, Rong Chen
diff --git a/include/linux/mm.h b/include/linux/mm.h index 8ba434287387..3568836841f9 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2780,7 +2780,6 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address, #define FOLL_NOWAIT 0x20 /* if a disk transfer is needed, start the IO * and return without waiting upon it */ #define FOLL_POPULATE 0x40 /* fault in page */ -#define FOLL_SPLIT 0x80 /* don't return transhuge pages, split them */ #define FOLL_HWPOISON 0x100 /* check page is hwpoisoned */ #define FOLL_NUMA 0x200 /* force NUMA hinting page fault */ #define FOLL_MIGRATION 0x400 /* wait for page to replace migration entry */ diff --git a/mm/gup.c b/mm/gup.c index e40579624f10..f3d45a8f18ae 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -435,18 +435,6 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, } } - if (flags & FOLL_SPLIT && PageTransCompound(page)) { - get_page(page); - pte_unmap_unlock(ptep, ptl); - lock_page(page); - ret = split_huge_page(page); - unlock_page(page); - put_page(page); - if (ret) - return ERR_PTR(ret); - goto retry; - } - /* try_grab_page() does nothing unless FOLL_GET or FOLL_PIN is set. */ if (unlikely(!try_grab_page(page, flags))) { page = ERR_PTR(-ENOMEM); @@ -591,7 +579,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, spin_unlock(ptl); return follow_page_pte(vma, address, pmd, flags, &ctx->pgmap); } - if (flags & (FOLL_SPLIT | FOLL_SPLIT_PMD)) { + if (flags & FOLL_SPLIT_PMD) { int ret; page = pmd_page(*pmd); if (is_huge_zero_page(page)) { @@ -600,19 +588,7 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, split_huge_pmd(vma, pmd, address); if (pmd_trans_unstable(pmd)) ret = -EBUSY; - } else if (flags & FOLL_SPLIT) { - if (unlikely(!try_get_page(page))) { - spin_unlock(ptl); - return ERR_PTR(-ENOMEM); - } - spin_unlock(ptl); - lock_page(page); - ret = split_huge_page(page); - unlock_page(page); - put_page(page); - if (pmd_none(*pmd)) - return no_page_table(vma, flags); - } else { /* flags & FOLL_SPLIT_PMD */ + } else { spin_unlock(ptl); split_huge_pmd(vma, pmd, address); ret = pte_alloc(mm, pmd) ? -ENOMEM : 0;
Since commit 5a52c9df62b4 ("uprobe: use FOLL_SPLIT_PMD instead of FOLL_SPLIT") and commit ba925fa35057 ("s390/gmap: improve THP splitting") FOLL_SPLIT has not been used anymore. Remove the dead code. Signed-off-by: Yang Shi <shy828301@gmail.com> --- include/linux/mm.h | 1 - mm/gup.c | 28 ++-------------------------- 2 files changed, 2 insertions(+), 27 deletions(-)