| Message ID | 20220228235741.102941-5-shy828301@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Make khugepaged collapse readonly FS THP more consistent | expand |
Hi Yang,
url: https://github.com/0day-ci/linux/commits/Yang-Shi/Make-khugepaged-collapse-readonly-FS-THP-more-consistent/20220301-075903
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: arm64-randconfig-m031-20220227 (https://download.01.org/0day-ci/archive/20220302/202203020034.2Ii9kTrs-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177)
mm/khugepaged.c:468 hugepage_vma_check() error: we previously assumed 'vma->vm_file' could be null (see line 455)
include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177)
vim +179 include/linux/huge_mm.h
2224ed1155c07b Yang Shi 2022-02-28 175 static inline bool file_thp_enabled(struct vm_area_struct *vma)
2224ed1155c07b Yang Shi 2022-02-28 176 {
2224ed1155c07b Yang Shi 2022-02-28 @177 struct inode *inode = vma->vm_file->f_inode;
^^^^^^^^^^^^^^
Dereference.
2224ed1155c07b Yang Shi 2022-02-28 178
2224ed1155c07b Yang Shi 2022-02-28 @179 return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) && vma->vm_file &&
^^^^^^^^^^^^
Checked too late.
2224ed1155c07b Yang Shi 2022-02-28 180 (vma->vm_flags & VM_EXEC) &&
2224ed1155c07b Yang Shi 2022-02-28 181 !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
2224ed1155c07b Yang Shi 2022-02-28 182 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 2022/3/3 19:43, Dan Carpenter wrote: > Hi Yang, > > url: https://github.com/0day-ci/linux/commits/Yang-Shi/Make-khugepaged-collapse-readonly-FS-THP-more-consistent/20220301-075903 > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev > config: arm64-randconfig-m031-20220227 (https://download.01.org/0day-ci/archive/20220302/202203020034.2Ii9kTrs-lkp@intel.com/config) > compiler: aarch64-linux-gcc (GCC) 11.2.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > smatch warnings: > include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177) > mm/khugepaged.c:468 hugepage_vma_check() error: we previously assumed 'vma->vm_file' could be null (see line 455) > include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177) > > vim +179 include/linux/huge_mm.h > > 2224ed1155c07b Yang Shi 2022-02-28 175 static inline bool file_thp_enabled(struct vm_area_struct *vma) > 2224ed1155c07b Yang Shi 2022-02-28 176 { > 2224ed1155c07b Yang Shi 2022-02-28 @177 struct inode *inode = vma->vm_file->f_inode; > ^^^^^^^^^^^^^^ > Dereference. > > 2224ed1155c07b Yang Shi 2022-02-28 178 > 2224ed1155c07b Yang Shi 2022-02-28 @179 return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) && vma->vm_file && > ^^^^^^^^^^^^ > Checked too late. Yep. We should check vma->vm_file first before we access vma->vm_file->f_inode. Thanks. > > 2224ed1155c07b Yang Shi 2022-02-28 180 (vma->vm_flags & VM_EXEC) && > 2224ed1155c07b Yang Shi 2022-02-28 181 !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode); > 2224ed1155c07b Yang Shi 2022-02-28 182 } > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org > > . >
On Thu, Mar 3, 2022 at 3:48 AM Miaohe Lin <linmiaohe@huawei.com> wrote: > > On 2022/3/3 19:43, Dan Carpenter wrote: > > Hi Yang, > > > > url: https://github.com/0day-ci/linux/commits/Yang-Shi/Make-khugepaged-collapse-readonly-FS-THP-more-consistent/20220301-075903 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev > > config: arm64-randconfig-m031-20220227 (https://download.01.org/0day-ci/archive/20220302/202203020034.2Ii9kTrs-lkp@intel.com/config) > > compiler: aarch64-linux-gcc (GCC) 11.2.0 > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > smatch warnings: > > include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177) > > mm/khugepaged.c:468 hugepage_vma_check() error: we previously assumed 'vma->vm_file' could be null (see line 455) > > include/linux/huge_mm.h:179 file_thp_enabled() warn: variable dereferenced before check 'vma->vm_file' (see line 177) > > > > vim +179 include/linux/huge_mm.h > > > > 2224ed1155c07b Yang Shi 2022-02-28 175 static inline bool file_thp_enabled(struct vm_area_struct *vma) > > 2224ed1155c07b Yang Shi 2022-02-28 176 { > > 2224ed1155c07b Yang Shi 2022-02-28 @177 struct inode *inode = vma->vm_file->f_inode; > > ^^^^^^^^^^^^^^ > > Dereference. > > > > 2224ed1155c07b Yang Shi 2022-02-28 178 > > 2224ed1155c07b Yang Shi 2022-02-28 @179 return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) && vma->vm_file && > > ^^^^^^^^^^^^ > > Checked too late. > > Yep. We should check vma->vm_file first before we access vma->vm_file->f_inode. Ah, yes, thanks for the report and the suggestion. > > Thanks. > > > > > 2224ed1155c07b Yang Shi 2022-02-28 180 (vma->vm_flags & VM_EXEC) && > > 2224ed1155c07b Yang Shi 2022-02-28 181 !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode); > > 2224ed1155c07b Yang Shi 2022-02-28 182 } > > > > --- > > 0-DAY CI Kernel Test Service, Intel Corporation > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org > > > > . > > >
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index e4c18ba8d3bf..e6d867f72458 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -172,6 +172,15 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma) return false; } +static inline bool file_thp_enabled(struct vm_area_struct *vma) +{ + struct inode *inode = vma->vm_file->f_inode; + + return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) && vma->vm_file && + (vma->vm_flags & VM_EXEC) && + !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode); +} + bool transparent_hugepage_active(struct vm_area_struct *vma); #define transparent_hugepage_use_zero_page() \ diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 406a3c28c026..a87b3df63209 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -64,13 +64,6 @@ static atomic_t huge_zero_refcount; struct page *huge_zero_page __read_mostly; unsigned long huge_zero_pfn __read_mostly = ~0UL; -static inline bool file_thp_enabled(struct vm_area_struct *vma) -{ - return transhuge_vma_enabled(vma, vma->vm_flags) && vma->vm_file && - !inode_is_open_for_write(vma->vm_file->f_inode) && - (vma->vm_flags & VM_EXEC); -} - bool transparent_hugepage_active(struct vm_area_struct *vma) { /* The addr is used to check if the vma size fits */ @@ -82,8 +75,8 @@ bool transparent_hugepage_active(struct vm_area_struct *vma) return __transparent_hugepage_enabled(vma); if (vma_is_shmem(vma)) return shmem_huge_enabled(vma); - if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS)) - return file_thp_enabled(vma); + if (transhuge_vma_enabled(vma, vma->vm_flags) && file_thp_enabled(vma)) + return true; return false; } diff --git a/mm/khugepaged.c b/mm/khugepaged.c index a0e4fa33660e..3dbac3e23f43 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -465,13 +465,8 @@ static bool hugepage_vma_check(struct vm_area_struct *vma, return false; /* Only regular file is valid */ - if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && vma->vm_file && - (vm_flags & VM_EXEC)) { - struct inode *inode = vma->vm_file->f_inode; - - return !inode_is_open_for_write(inode) && - S_ISREG(inode->i_mode); - } + if (file_thp_enabled(vma)) + return true; if (!vma->anon_vma || vma->vm_ops) return false;
Since commit a4aeaa06d45e ("mm: khugepaged: skip huge page collapse for special files"), khugepaged just collapses THP for regular file which is the intended usecase for readonly fs THP. Only show regular file as THP eligible accordingly. And make file_thp_enabled() available for khugepaged too in order to remove duplicate code. Signed-off-by: Yang Shi <shy828301@gmail.com> --- include/linux/huge_mm.h | 9 +++++++++ mm/huge_memory.c | 11 ++--------- mm/khugepaged.c | 9 ++------- 3 files changed, 13 insertions(+), 16 deletions(-)