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. > @@ -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? > @@ -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. 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. Thanks. -- tejun _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
