Re: [PATCH v2 09/20] mm, hugetlb: protect region tracking via newlyintroduced resv_map lock

[Joonsoo Kim]

> > @@ -202,15 +199,27 @@ 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) {
> > +			nrg = kmalloc(sizeof(*nrg), GFP_NOWAIT);
> > +			if (!nrg) {
> > +				spin_unlock(&resv->lock);
> > +				nrg = kmalloc(sizeof(*nrg), GFP_KERNEL);
> > +				if (!nrg) {
> > +					chg = -ENOMEM;
> > +					goto out;
> 
> Just return -ENOMEM here.

Okay. It looks better!

> 
> > +				}
> > +				goto retry;
> > +			}
> > +		}
> > +
> 
> You seem to be right, at least in my workloads, the hold times for the
> region lock is quite small, so a spinlock is better than a sleeping
> lock.
> 
> That said, this code is quite messy, but I cannot think of a
> better/cleaner approach right now.

Okay.

Thanks for review!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/