On Wed, Oct 26, 2016 at 04:07:25PM -0400, Tejun Heo wrote: > Hello, Daniel. > > On Wed, Oct 26, 2016 at 09:25:25PM +0200, Daniel Vetter wrote: > > > > + * Note that callers must ensure that concurrent access to @ida is not possible. > > > > + * When simplicity trumps concurrency needs look at ida_simple_get() instead. > > > > > > Maybe we can make it a bit less dramatic? > > > > What about? > > > > "Note that callers must ensure that concurrent access to @ida is not possible. > > See ida_simple_get() for a varaint which takes care of locking. > > Yeah, that reads easier to me. > > > > Hmm... so, this isn't necessarily about speed. For example, id > > > allocation might have to happen inside a spinlock which protects a > > > larger scope. To guarantee GFP_KERNEL allocation behavior in such > > > cases, the caller would have to call ida_pre_get() outside the said > > > spinlock and then call ida_get_new_above() inside the lock. > > > > Hm, ida_simple_get does that for you already ... > > Here's an example. > > spin_lock(); > do some stuff; > something->id = ida_simple_get(some gfp flag); > do some stuff; > spin_unlock(); > > In this scenario, you can't use sleeping GFPs for ida_simple_get() > because it does preloading inside it. What one has to do is... > > ida_pre_get(GFP_KERNEL); > spin_lock(); > do some stuff; > something->id = ida_get_new_above(GFP_NOWAIT); > do some stuff; > spin_unlock(); > > So, I guess it can be sometimes about avoiding the extra locking > overhead but it's more often about separating out allocation context > into an earlier call. Hm yeah, calling ida_simple_get in that case isn't recommend really. > > > I think it'd be better to explain what the simple version does and > > > expects and then say that unless there are specific requirements using > > > the simple version is recommended. > > > > What about: > > > > "Compared to ida_get_new_above() this function does its own locking, and > > should be used unless there are special requirements." > > Yeah, looks good to me. Ok, will respin, thanks for the review comments. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
