Trond Myklebust writes:
 > In NFSv4 we often want to serialize asynchronous RPC calls with ordinary

[...]

 > +
 > +void fastcall iosem_lock(struct iosem *lk)
 > +{
 > +    struct iosem_wait waiter;
 > +
 > +    might_sleep();
 > +
 > +    init_iosem_waiter(&waiter);
 > +    waiter.wait.func = iosem_lock_wake_function;
 > +
 > +    set_current_state(TASK_UNINTERRUPTIBLE);
 > +    if (__iosem_lock(lk, &waiter))
 > +            schedule();
 > +    __set_current_state(TASK_RUNNING);
 > +
 > +    BUG_ON(!list_empty(&waiter.wait.task_list));
 > +}
 > +EXPORT_SYMBOL(iosem_lock);

As I understand it, in the blocking path IOSEM_LOCK_EXCLUSIVE is set by
iosem_lock_wake_function() called by the waker thread. But this is
asking for convoy formation: iosem_unlock() transfers ownership of the
lock to the thread that is currently sleeping. This means that all
threads _running_ on another processors and bumping into that lock will
go to sleep too (i.e., lock is owned but unused), thus forming a
"convoy" that has a tendency to grow over time when there is at least
smallest contention. This is known problem with all "early ownership
transfer" locks designs (except maybe in your case contention is not
supposed to happen).

And as a nitpick: struct iosem is emphatically _not_ a semaphore, it
even doesn't have a counter. :) Can it be named iomutex or iolock or
async_lock or something? We have enough confusion going on with struct
semaphore that is mostly used as mutex.

[...]

 > +
 > +int fastcall iosem_lock_and_schedule_work(struct iosem *lk, struct 
 > iosem_work *wk)
 > +{
 > +    int ret;
 > +
 > +    init_iosem_waiter(&wk->waiter);
 > +    wk->waiter.wait.func = iosem_lock_and_schedule_function;
 > +    ret = __iosem_lock(lk, &wk->waiter);
 > +    if (ret == 0)
 > +            ret = schedule_work(&wk->work);
 > +    return ret;
 > +}

This is actually trylock, right? If iosem_lock_and_schedule_work()
returns -EINPROGRESS lock is not acquired on return and caller has to
call schedule().

[...]

 > 
 > -- 
 > Trond Myklebust <[EMAIL PROTECTED]>
 > 

Nikita.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to