Re: [PATCH v6 17/25] rnbd: client: main functionality

Roman Penyaev
2020-01-10T15:09:47Z

On 2020-01-10 15:45, Jinpu Wang wrote:
>> > +{
>> > + DEFINE_WAIT_FUNC(wait, autoremove_wake_function);
>> > +
>> > + prepare_to_wait(&sess->rtrs_waitq, &wait, TASK_UNINTERRUPTIBLE);
>> > + if (IS_ERR_OR_NULL(sess->rtrs)) {
>> > + finish_wait(&sess->rtrs_waitq, &wait);
>> > + return;
>> > + }
>> > + mutex_unlock(&sess_lock);
>> > + /* After unlock session can be freed, so careful */
>> > + schedule();
>> > + mutex_lock(&sess_lock);
>> > +}
>>
>> How can a function that calls schedule() and that is not surrounded by
>> a
>> loop be correct? What if e.g. schedule() finishes due to a spurious
>> wakeup?
> I checked in git history, this no clean explanation why we have to
> call the mutex_unlock/schedul/mutex_lock magic
> It's allowed to call schedule inside mutex, seems we can remove the
> code snip, @Roman Penyaev do you remember why it was introduced?

The loop in question is in the caller, see __find_and_get_sess().
You can't leave mutex locked and call schedule(), you will catch a
deadlock with a caller of free_sess(), which has just put the last
reference and is about to take the sess_lock in order to delete
the session from the list.

--
Roman