linux-kernel - Re: [PATCH v2 1/2] Revert "locking/rwsem: Remove reader optimistic spinning"

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]

Message-ID: <20230906112741.GA18415@KORCO045595.samsungds.net>
Date:   Wed, 6 Sep 2023 20:27:41 +0900
From:   Bongkyu Kim <bongkyu7.kim@...sung.com>
To:     Waiman Long <longman@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>, mingo@...hat.com,
        will@...nel.org, boqun.feng@...il.com,
        linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org
Subject: Re: [PATCH v2 1/2] Revert
 "locking/rwsem: Remove reader optimistic spinning"

On Mon, Sep 04, 2023 at 03:56:56PM -0400, Waiman Long wrote:
> On 9/4/23 11:10, Peter Zijlstra wrote:
> > On Fri, Sep 01, 2023 at 10:07:03AM +0900, Bongkyu Kim wrote:
> > > This reverts commit 617f3ef95177840c77f59c2aec1029d27d5547d6.
> > > 
> > > In mobile environment, reader optimistic spinning is still useful
> > > because there're not many readers. In my test result at android device,
> > > it improves application startup time about 3.8%
> > > App startup time is most important factor for android user expriences.
> > > So, re-enable reader optimistic spinning by this commit. And,
> > > the later patch will make it optional feature by cmdline.
> > I'm not seeing any mention on how this interacts with all the rwsem work
> > that has been done since that commit, like the handoff rework.
> > 
> > Why is a straight revert a sane thing at this point?
> 
> I also agree that a revert is not the best way to reintroduce the feature.
> It should document the reason why reader optimistic spinning is not the
> default as discussed in commit 617f3ef9517 ("locking/rwsem: Remove reader
> optimistic spinning") and under what condition should reader optimistic
> spinning can be turned back on.
> 
> Besides, I now think we may not really need 2 separate nonspinnable bits. We
> can go with one that is set by writer timing out when spinning on reader.
> 
> Cheers,
> Longman

Should I modify like the below?
- Title to "locking/rwsem: Reintroduce reader optimistic spinning"
- Add more document like Longman's comment
- Reconsidering about 2 separate nonspinnable bits to one

Thanks,
Bongkyu