Messages in this thread | | | Date | Mon, 27 Jan 2014 16:02:56 -0500 | From | Naoya Horiguchi <> | Subject | Re: [PATCH 3/8] mm, hugetlb: fix race in region tracking |
| |
On Sun, Jan 26, 2014 at 07:52:21PM -0800, Davidlohr Bueso wrote: > From: Joonsoo Kim <iamjoonsoo.kim@lge.com> > > There is a race condition if we map a same file on different processes. > Region tracking is protected by mmap_sem and hugetlb_instantiation_mutex. > When we do mmap, we don't grab a hugetlb_instantiation_mutex, but only the, > mmap_sem (exclusively). This doesn't prevent other tasks from modifying the > region structure, so it can be modified by two processes concurrently. > > To solve this, introduce a spinlock to resv_map and make region manipulation > function grab it before they do actual work. > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> > [Updated changelog] > Signed-off-by: Davidlohr Bueso <davidlohr@hp.com> > --- ... > @@ -203,15 +200,23 @@ static long region_chg(struct resv_map *resv, long f, long t) > * Subtle, allocate a new region at the position but make it zero > * size such that we can guarantee to record the reservation. */ > if (&rg->link == head || t < rg->from) { > - nrg = kmalloc(sizeof(*nrg), GFP_KERNEL); > - if (!nrg) > - return -ENOMEM; > + if (!nrg) { > + spin_unlock(&resv->lock);
I think that doing kmalloc() inside the lock is simpler. Why do you unlock and retry here?
Thanks, Naoya Horiguchi
> + nrg = kmalloc(sizeof(*nrg), GFP_KERNEL); > + if (!nrg) > + return -ENOMEM; > + > + goto retry; > + } > + > nrg->from = f; > nrg->to = f; > INIT_LIST_HEAD(&nrg->link); > list_add(&nrg->link, rg->link.prev); > + nrg = NULL; > > - return t - f; > + chg = t - f; > + goto out_locked; > } > > /* Round our left edge to the current segment if it encloses us. */
| |