Messages in this thread | | | Date | Wed, 18 Jul 2012 14:26:28 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] hugetlb/cgroup: Simplify pre_destroy callback |
| |
On Wed, 18 Jul 2012 11:04:09 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > Since we cannot fail in hugetlb_cgroup_move_parent, we don't really > need to check whether cgroup have any change left after that. Also skip > those hstates for which we don't have any charge in this cgroup. > > ... > > + for_each_hstate(h) { > + /* > + * if we don't have any charge, skip this hstate > + */ > + idx = hstate_index(h); > + if (res_counter_read_u64(&h_cg->hugepage[idx], RES_USAGE) == 0) > + continue; > + spin_lock(&hugetlb_lock); > + list_for_each_entry(page, &h->hugepage_activelist, lru) > + hugetlb_cgroup_move_parent(idx, cgroup, page); > + spin_unlock(&hugetlb_lock); > + VM_BUG_ON(res_counter_read_u64(&h_cg->hugepage[idx], RES_USAGE)); > + } > out: > return ret; > }
This looks fishy.
We test RES_USAGE before taking hugetlb_lock. What prevents some other thread from increasing RES_USAGE after that test?
After walking the list we test RES_USAGE after dropping hugetlb_lock. What prevents another thread from incrementing RES_USAGE before that test, triggering the BUG?
| |