On Tue, 28 Jan 2014 17:19:38 -0800 Davidlohr Bueso <davidlohr@xxxxxx> 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.... -- 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>