Re: [PATCH v3 1/3] rtmutex: update rt-mutex-design

From: Steven Rostedt
Date: Thu Jul 06 2017 - 09:25:37 EST


On Thu, 6 Jul 2017 10:39:28 +0800
Alex Shi <alex.shi@xxxxxxxxxx> wrote:

> Hi Steven,
>
> Thanks a lot for detailed review. Every suggestion were token except one need
> extra review: the 'Waking up in loop'. Is this OK or need more further change?
>
> BTW, I didn't add you on Reviewers, since you are author already. :)

Actually, I probably should be. Comment below.

>
>
> Best regards
> Alex
>
>
> On 07/04/2017 02:49 AM, Steven Rostedt wrote:
> >> +In the first case, the task will try again to acquire the lock. If it
> >
> > Hmm, I know you mention it below, but it is confusing. In both cases
> > the task will try again to acquire the lock. The difference between the
> > two cases is what happens if it fails to acquire the lock.
> >
> > This part should be rewritten.
> >
>
> +The task can then wake up for a couple of reasons:
> + 1) The previous lock owner released the lock, and the task now is top_waiter
> + 2) we received a signal or timeout
>
> +In both cases, the task will try again to acquire the lock. If it
> +does, then it will take itself off the waiters tree and set itself back
> +to the TASK_RUNNING state.
>
> +In first case, if the lock was acquired by another task before this task
> +could get the lock, then it will go back to sleep and wait to be woken again.
>
> +The second case is only applicable for tasks that are grabbing a mutex
> +that can wake up before getting the lock, either due to a signal or
> +a timeout (i.e. rt_mutex_timed_futex_lock()). When woken, it will try to
> +take the lock again, if it succeeds, then the task will return with the
> +lock held, otherwise it will return with -EINTR if the task was woken
> +by a signal, or -ETIMEDOUT if it timed out.

This looks fine.

>
> ....
>
> -Reviewers: Ingo Molnar, Thomas Gleixner, Thomas Duetsch, and Randy Dunlap
> +Reviewers: Ingo Molnar, Thomas Gleixner, Thomas Duetsch, Randy Dunlap
> + and Sebastian Siewior

Since the updated was not reviewed by all of the above, you should
change this to:

Original Reviewers: Ingo Molnar, Thomas Gleixner, Thomas Deutsch, and
Randy Dunlap

Update (7/6/2017) Reviewers: Steven Rostedt and Sebastian Siewior

Did Randy make comments?

-- Steve