On Sun, Jan 26, 2014 at 07:52:21PM -0800, Davidlohr Bueso wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> [Updated changelog]
> Signed-off-by: Davidlohr Bueso <davidlohr@xxxxxx>
> ---
...
> @@ -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. */
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>