On Wed, Oct 26, 2016 at 10:39:29AM -0400, Tejun Heo wrote: > Hello, Daniel. > > On Wed, Oct 26, 2016 at 04:27:39PM +0200, Daniel Vetter wrote: > > I wanted to wrap a bunch of ida_simple_get calls into their own > > locking, until I dug around and read the original commit message. > > Stuff like this should imo be added to the kernel doc, let's do that. > > Generally agreed but some nits below. I value good docs but I suck at typing them ;-) > > @@ -927,6 +927,9 @@ EXPORT_SYMBOL(ida_pre_get); > > * and go back to the ida_pre_get() call. If the ida is full, it will > > * return %-ENOSPC. > > * > > + * 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. > > > > @@ -1073,6 +1076,10 @@ EXPORT_SYMBOL(ida_destroy); > > * Allocates an id in the range start <= id < end, or returns -ENOSPC. > > * On memory allocation failure, returns -ENOMEM. > > * > > + * Compared to ida_get_new_above() this function does its own locking and hence > > + * is recommended everywhere where simplicity is preferred over the last bit of > > + * speed. > > 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 ... > 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." -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
