Re: [kbuild-all] Re: [PATCH] mm: gup: remove FOLL_SPLIT
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 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
Re: [PATCH] mm: gup: remove FOLL_SPLIT
On Tue, Mar 30, 2021 at 12:08 AM John Hubbard 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 > > --- > > 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 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, >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; > > >
Re: [PATCH] mm: gup: remove FOLL_SPLIT
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 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, -- John Hubbard NVIDIA
Re: [PATCH] mm: gup: remove FOLL_SPLIT
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 --- 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 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_MIGRATION0x400 /* 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, >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;
Re: [PATCH] mm: gup: remove FOLL_SPLIT
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 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-...@lists.01.org .config.gz Description: application/gzip