On Fri, Jan 10, 2020 at 4:09 PM Roman Penyaev <rpenyaev@xxxxxxx> wrote:
>
> 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
Thanks Roman for quick reply, will extend the comment
