Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Wed, 20 Sep 2023 13:09:08 +0900
From:   Bongkyu Kim <bongkyu7.kim@samsung.com>
To:     Waiman Long <longman@redhat.com>
Cc:     Peter Zijlstra <peterz@infradead.org>, mingo@redhat.com,
        will@kernel.org, boqun.feng@gmail.com,
        linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
        bongkyu7.kim@samsung.com
Subject: Re: [PATCH v2 1/2] Revert
 "locking/rwsem: Remove reader optimistic spinning"
Message-ID: <20230920040908.GA4197@KORCO045595.samsungds.net>
MIME-Version: 1.0
In-Reply-To: <9578a3a7-4151-6b60-3241-d883afe08bc1@redhat.com>
Content-Type: multipart/mixed;
        boundary="----fhEHFYu7RNsFUiSWfAq-IjQ-VzC.j9Pzsqf7lJgctIRm.Jgh=_8f6b_"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <20230901010704.18493-1-bongkyu7.kim@samsung.com>
        <CGME20230901010734epcas2p1a0d3698e76590637f9175fca40ad9eaf@epcas2p1.samsung.com>
        <20230901010704.18493-2-bongkyu7.kim@samsung.com>
        <20230904151008.GA25717@noisy.programming.kicks-ass.net>
        <94764456-c4d4-03eb-81ef-df402f4916f6@redhat.com>
        <20230906112741.GA18415@KORCO045595.samsungds.net>
        <9578a3a7-4151-6b60-3241-d883afe08bc1@redhat.com>
Precedence: bulk

------fhEHFYu7RNsFUiSWfAq-IjQ-VzC.j9Pzsqf7lJgctIRm.Jgh=_8f6b_
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline

On Wed, Sep 06, 2023 at 09:32:45AM -0400, Waiman Long wrote:
> 
> On 9/6/23 07:27, Bongkyu Kim wrote:
> > 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
> 
> Besides the above, Peter also ask to verify that it won't affect handoff
> handling which requires that an unlocker see the lock will be free and wake
> up the head of the wait queue. Given the fact that the simple heuristic of
> skipping optimistic spinning if the lock is reader owned is kept, that
> shouldn't be a problem, but you still need to document that.
> 
> Cheers,
> Longman
 
I've been reviewing nonspinnable bits for several days, but I can't find the
way for one spinnable bit. In this patch, how about modify only already mentioned
document part? About one spinnable bit, we will discuss later.

------fhEHFYu7RNsFUiSWfAq-IjQ-VzC.j9Pzsqf7lJgctIRm.Jgh=_8f6b_
Content-Type: text/plain; charset="utf-8"


------fhEHFYu7RNsFUiSWfAq-IjQ-VzC.j9Pzsqf7lJgctIRm.Jgh=_8f6b_--