Hi Miaohe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210125]
[also build test ERROR on v5.11-rc7]
[cannot apply to linux/master linus/master hnaz-linux-mm/master v5.11-rc7 
v5.11-rc6 v5.11-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    
https://github.com/0day-ci/linux/commits/Miaohe-Lin/hugetlb_cgroup-fix-imbalanced-css_get-and-css_put-pair-for-shared-mappings/20210210-171736
base:    59fa6a163ffabc1bf25c5e0e33899e268a96d3cc
config: x86_64-randconfig-c002-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # 
https://github.com/0day-ci/linux/commit/68f4ed1b80aa7c51a921c3ad913ee7e6e00618d0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review 
Miaohe-Lin/hugetlb_cgroup-fix-imbalanced-css_get-and-css_put-pair-for-shared-mappings/20210210-171736
        git checkout 68f4ed1b80aa7c51a921c3ad913ee7e6e00618d0
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

   mm/hugetlb.c: In function 'hugetlb_reserve_pages':
>> mm/hugetlb.c:5222:17: error: dereferencing pointer to incomplete type 
>> 'struct hugetlb_cgroup'
    5222 |    css_put(&h_cg->css);
         |                 ^~


vim +5222 mm/hugetlb.c

  5091  
  5092  /* Return true if reservation was successful, false otherwise.  */
  5093  bool hugetlb_reserve_pages(struct inode *inode,
  5094                                          long from, long to,
  5095                                          struct vm_area_struct *vma,
  5096                                          vm_flags_t vm_flags)
  5097  {
  5098          long chg, add = -1;
  5099          struct hstate *h = hstate_inode(inode);
  5100          struct hugepage_subpool *spool = subpool_inode(inode);
  5101          struct resv_map *resv_map;
  5102          struct hugetlb_cgroup *h_cg = NULL;
  5103          long gbl_reserve, regions_needed = 0;
  5104  
  5105          /* This should never happen */
  5106          if (from > to) {
  5107                  VM_WARN(1, "%s called with a negative range\n", 
__func__);
  5108                  return false;
  5109          }
  5110  
  5111          /*
  5112           * Only apply hugepage reservation if asked. At fault time, an
  5113           * attempt will be made for VM_NORESERVE to allocate a page
  5114           * without using reserves
  5115           */
  5116          if (vm_flags & VM_NORESERVE)
  5117                  return true;
  5118  
  5119          /*
  5120           * Shared mappings base their reservation on the number of 
pages that
  5121           * are already allocated on behalf of the file. Private 
mappings need
  5122           * to reserve the full area even if read-only as mprotect() may 
be
  5123           * called to make the mapping read-write. Assume !vma is a shm 
mapping
  5124           */
  5125          if (!vma || vma->vm_flags & VM_MAYSHARE) {
  5126                  /*
  5127                   * resv_map can not be NULL as hugetlb_reserve_pages is 
only
  5128                   * called for inodes for which resv_maps were created 
(see
  5129                   * hugetlbfs_get_inode).
  5130                   */
  5131                  resv_map = inode_resv_map(inode);
  5132  
  5133                  chg = region_chg(resv_map, from, to, &regions_needed);
  5134  
  5135          } else {
  5136                  /* Private mapping. */
  5137                  resv_map = resv_map_alloc();
  5138                  if (!resv_map)
  5139                          return false;
  5140  
  5141                  chg = to - from;
  5142  
  5143                  set_vma_resv_map(vma, resv_map);
  5144                  set_vma_resv_flags(vma, HPAGE_RESV_OWNER);
  5145          }
  5146  
  5147          if (chg < 0)
  5148                  goto out_err;
  5149  
  5150          if (hugetlb_cgroup_charge_cgroup_rsvd(hstate_index(h),
  5151                                  chg * pages_per_huge_page(h), &h_cg) < 
0)
  5152                  goto out_err;
  5153  
  5154          if (vma && !(vma->vm_flags & VM_MAYSHARE) && h_cg) {
  5155                  /* For private mappings, the hugetlb_cgroup uncharge 
info hangs
  5156                   * of the resv_map.
  5157                   */
  5158                  resv_map_set_hugetlb_cgroup_uncharge_info(resv_map, 
h_cg, h);
  5159          }
  5160  
  5161          /*
  5162           * There must be enough pages in the subpool for the mapping. If
  5163           * the subpool has a minimum size, there may be some global
  5164           * reservations already in place (gbl_reserve).
  5165           */
  5166          gbl_reserve = hugepage_subpool_get_pages(spool, chg);
  5167          if (gbl_reserve < 0)
  5168                  goto out_uncharge_cgroup;
  5169  
  5170          /*
  5171           * Check enough hugepages are available for the reservation.
  5172           * Hand the pages back to the subpool if there are not
  5173           */
  5174          if (hugetlb_acct_memory(h, gbl_reserve) < 0)
  5175                  goto out_put_pages;
  5176  
  5177          /*
  5178           * Account for the reservations made. Shared mappings record 
regions
  5179           * that have reservations as they are shared by multiple VMAs.
  5180           * When the last VMA disappears, the region map says how much
  5181           * the reservation was and the page cache tells how much of
  5182           * the reservation was consumed. Private mappings are per-VMA 
and
  5183           * only the consumed reservations are tracked. When the VMA
  5184           * disappears, the original reservation is the VMA size and the
  5185           * consumed reservations are stored in the map. Hence, nothing
  5186           * else has to be done for private mappings here
  5187           */
  5188          if (!vma || vma->vm_flags & VM_MAYSHARE) {
  5189                  add = region_add(resv_map, from, to, regions_needed, h, 
h_cg);
  5190  
  5191                  if (unlikely(add < 0)) {
  5192                          hugetlb_acct_memory(h, -gbl_reserve);
  5193                          goto out_put_pages;
  5194                  } else if (unlikely(chg > add)) {
  5195                          /*
  5196                           * pages in this range were added to the reserve
  5197                           * map between region_chg and region_add.  This
  5198                           * indicates a race with alloc_huge_page.  
Adjust
  5199                           * the subpool and reserve counts modified above
  5200                           * based on the difference.
  5201                           */
  5202                          long rsv_adjust;
  5203  
  5204                          /*
  5205                           * hugetlb_cgroup_uncharge_cgroup_rsvd() will 
put the
  5206                           * reference to h_cg->css. See comment below 
for detail.
  5207                           */
  5208                          hugetlb_cgroup_uncharge_cgroup_rsvd(
  5209                                  hstate_index(h),
  5210                                  (chg - add) * pages_per_huge_page(h), 
h_cg);
  5211  
  5212                          rsv_adjust = hugepage_subpool_put_pages(spool,
  5213                                                                  chg - 
add);
  5214                          hugetlb_acct_memory(h, -rsv_adjust);
  5215                  } else if (h_cg) {
  5216                          /*
  5217                           * The file_regions will hold their own 
reference to
  5218                           * h_cg->css. So we should release the 
reference held
  5219                           * via hugetlb_cgroup_charge_cgroup_rsvd() when 
we are
  5220                           * done.
  5221                           */
> 5222                          css_put(&h_cg->css);
  5223                  }
  5224          }
  5225          return true;
  5226  
  5227  out_put_pages:
  5228          /* put back original number of pages, chg */
  5229          (void)hugepage_subpool_put_pages(spool, chg);
  5230  out_uncharge_cgroup:
  5231          hugetlb_cgroup_uncharge_cgroup_rsvd(hstate_index(h),
  5232                                              chg * 
pages_per_huge_page(h), h_cg);
  5233  out_err:
  5234          if (!vma || vma->vm_flags & VM_MAYSHARE)
  5235                  /* Only call region_abort if the region_chg succeeded 
but the
  5236                   * region_add failed or didn't run.
  5237                   */
  5238                  if (chg >= 0 && add < 0)
  5239                          region_abort(resv_map, from, to, 
regions_needed);
  5240          if (vma && is_vma_resv_set(vma, HPAGE_RESV_OWNER))
  5241                  kref_put(&resv_map->refs, resv_map_release);
  5242          return false;
  5243  }
  5244  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachment: .config.gz
Description: application/gzip

Reply via email to