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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Jul 2017 09:25:23 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Alex Shi <alex.shi@...aro.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        "open list:LOCKING PRIMITIVES" <linux-kernel@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Sebastian Siewior <bigeasy@...utronix.de>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Juri Lelli <juri.lelli@....com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v3 1/3] rtmutex: update rt-mutex-design

On Thu, 6 Jul 2017 10:39:28 +0800
Alex Shi <alex.shi@...aro.org> 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