On Sat, Feb 9, 2013 at 3:42 PM, Michel Lespinasse <wal...@google.com> wrote: > On Fri, Feb 8, 2013 at 11:30 PM, Hillf Danton <dhi...@gmail.com> wrote: >> On Sat, Feb 9, 2013 at 10:45 AM, Michel Lespinasse <wal...@google.com> wrote: >>> + if (waiter->type != RWSEM_WAITING_FOR_WRITE) { >>> + list_del(&waiter->list); >>> + >>> + /* Set RWSEM_WAITING_BIAS before waking the last >>> reader >>> + * so we know there will be a remaining active >>> locker. >>> + */ >>> + if (!(--readers) && !list_empty(&sem->wait_list)) >>> + rwsem_atomic_add(RWSEM_WAITING_BIAS, sem); >>> + >>> + tsk = waiter->task; >>> + smp_mb(); >> >> For what with mb? > > This is the same mb that used to be before the readers_only label > before this change. > The intention is that the write to waiter->task must be the last > access to waiter - after that, the reader is granted the lock so it > could do anything, including deallocating the waiter structure as it > gets out of rwsem_down_read_failed() and possibly deallocating the > task structure if it exits > Perhaps the waiting process needs to see if no more wait needed, /* wait to be given the lock */ for (;;) { if (!waiter.task) break; schedule(); set_task_state(tsk, TASK_UNINTERRUPTIBLE); } and another mb do exist at the beginning of try_to_wake_up().
And I want to change + /* Block until there are no active lockers. */ + do { + schedule(); + set_task_state(tsk, TASK_UNINTERRUPTIBLE); + } while (sem->count & RWSEM_ACTIVE_MASK); to /* Block until there are no active lockers. */ while (sem->count & RWSEM_ACTIVE_MASK) { schedule(); set_task_state(tsk, TASK_UNINTERRUPTIBLE); } for sure that we have to wait. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/