On Wed, 13 Apr 2011 21:26:24 +0200, Daniel Vetter <[email protected]> wrote: > On Tue, Apr 12, 2011 at 09:31:52PM +0100, Chris Wilson wrote: > > Replace the three nearly identical copies of the code with a single > > function. And take advantage of the opportunity to do some > > micro-optimisation: avoid the vmalloc if at all possible and also avoid > > dropping the lock unless we are forced to acquire the mm semaphore. > > One tiny nitpick: Perhaps put an api comment at the top of > gem_get_user_pages that this function drops the struct_mutex. That's not > something we normally do and could cause endless amounts of fun if > neglected.
How about: /** * Magically retrieves the pages for the user addr whilst holding the * dev->struct_mutex. * * Since we can not take the mm semaphore whilst holding our dev->struct_mutex, * due to the pre-existing lock dependency established by i915_gem_fault(), * we have to perform some sleight-of-hand. * * First, we try the lockless variant of gup whilst continuing to hold the * mutex. If that fails to get all the user pages, then we no choice but * to acquire the mm semaphore (thus dropping the lock on dev->struct_mutex * to do so). The dev->struct_mutex is then re-acquired before we return. * * Returns: an error code *and* the number of user pages acquired. Even * on an error, you must iterate over the return pages and release them. */ ? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
