Messages in this thread | | | Date | Wed, 26 Oct 2016 10:39:29 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH] lib/ida: Document locking requirements a bit better |
| |
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
| |