Messages in this thread | | | Date | Mon, 3 Feb 2014 16:18:21 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 3/8] mm, hugetlb: fix race in region tracking |
| |
On Tue, 28 Jan 2014 17:19:38 -0800 Davidlohr Bueso <davidlohr@hp.com> wrote:
> On Tue, 2014-01-28 at 19:36 -0500, Naoya Horiguchi wrote: > > On Mon, Jan 27, 2014 at 06:34:17PM -0800, Davidlohr Bueso wrote: > [...] > > > > If this retry is really essential for the fix, please comment the reason > > > > both in patch description and inline comment. It's very important for > > > > future code maintenance. > > > > > > So we locate the corresponding region in the reserve map, and if we are > > > below the current region, then we allocate a new one. Since we dropped > > > the lock to allocate memory, we have to make sure that we still need the > > > new region and that we don't race with the new status of the reservation > > > map. This is the whole point of the retry, and I don't see it being > > > suboptimal. > > > > I'm afraid that you don't explain why you need drop the lock for memory > > allocation. Are you saying that this unlocking comes from the difference > > between rwsem and spin lock? > > Because you cannot go to sleep while holding a spinlock, which is > exactly what kmalloc(GFP_KERNEL) can do. We *might* get a way with it > with GFP_ATOMIC, I dunno, but I certainly prefer this approach better.
yup. You could do
foo = kmalloc(size, GFP_NOWAIT); if (!foo) { spin_unlock(...); foo = kmalloc(size, GFP_KERNEL); if (!foo) ... spin_lock(...); }
that avoids the lock/unlock once per allocation. But it also increases the lock's average hold times....
| |