Received-SPF: pass (google.com: domain of linux-kernel+bounces-132480-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <76954c9b-e87a-c530-edb7-a8be4e3327ab@samsung.com>
Date: Fri, 5 Apr 2024 15:37:30 +0900
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
	Thunderbird/102.15.1
Subject: Re: [PATCH v2 0/2] Make reader optimistic spinning optional
Content-Language: en-US
To: Waiman Long <longman@redhat.com>, John Stultz <jstultz@google.com>
Cc: 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
From: Bongkyu Kim <bongkyu7.kim@samsung.com>
In-Reply-To: <45ddde77-a5dd-41e8-933f-36ed0f8cf178@redhat.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <CGME20230901010734epcas2p4aadced02d68d3db407fda23de34601d2@epcas2p4.samsung.com>
	<20230901010704.18493-1-bongkyu7.kim@samsung.com>
	<CANDhNCoFRr=qizswLm-dzxJK0fHuCx98Z2B1pUspdwGqBEejYg@mail.gmail.com>
	<20240403012132.GA460@KORCO045595.samsungds.net>
	<CANDhNCpvKj6Swer-8DtQEotdnOiqfAg43oZLw_HZs6ogwqPwzg@mail.gmail.com>
	<20240403014207.GA499@KORCO045595.samsungds.net>
	<23fd78bb-76a7-46e8-9523-5d2cab4186f5@redhat.com>
	<45ddde77-a5dd-41e8-933f-36ed0f8cf178@redhat.com>

On 4/5/24 12:06, Waiman Long wrote:
> On 4/4/24 13:44, Waiman Long wrote:
>>
>> On 4/2/24 21:42, Bongkyu Kim wrote:
>>> On Tue, Apr 02, 2024 at 06:27:40PM -0700, John Stultz wrote:
>>>> On Tue, Apr 2, 2024 at 6:21 PM Bongkyu Kim
>>>> <bongkyu7.kim@samsung.com> wrote:
>>>>> On Tue, Apr 02, 2024 at 04:46:06PM -0700, John Stultz wrote:
>>>>>> On Thu, Aug 31, 2023 at 6:07 PM Bongkyu Kim
>>>>>> <bongkyu7.kim@samsung.com> wrote:
>>>>>>> This is rework of the following discussed patch.
>>>>>>> https://lore.kernel.org/all/20230613043308.GA1027@KORCO045595.samsungds.net/
>>>>>>>
>>>>>>> Changes from the previous patch
>>>>>>> - Split to revert and modify patches
>>>>>>> - Change according to Waiman Long's review
>>>>>>>      More wording to documentation part
>>>>>>>      Change module_param to early_param
>>>>>>>      Code change by Waiman Long's suggestion
>>>>>>>
>>>>>>> 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,
>>>>>>> make it optional feature by cmdline.
>>>>>>>
>>>>>>> Test result:
>>>>>>> This is 15 application startup performance in our exynos soc.
>>>>>>> - Cortex A78*2 + Cortex A55*6
>>>>>>> - unit: ms (lower is better)
>>>>>>>
>>>>>>> Application             base  opt_rspin  Diff  Diff(%)
>>>>>>> --------------------  ------  ---------  ----  -------
>>>>>>> * Total(geomean)         343        330   -13    +3.8%
>>>>>>> --------------------  ------  ---------  ----  -------
>>>>>>> helloworld               110        108    -2    +1.8%
>>>>>>> Amazon_Seller            397        388    -9    +2.3%
>>>>>>> Whatsapp                 311        304    -7    +2.3%
>>>>>>> Simple_PDF_Reader        500        463   -37    +7.4%
>>>>>>> FaceApp                  330        317   -13    +3.9%
>>>>>>> Timestamp_Camera_Free    451        443    -8    +1.8%
>>>>>>> Kindle                   629        597   -32    +5.1%
>>>>>>> Coinbase                 243        233   -10    +4.1%
>>>>>>> Firefox                  425        399   -26    +6.1%
>>>>>>> Candy_Crush_Soda         552        538   -14    +2.5%
>>>>>>> Hill_Climb_Racing        245        230   -15    +6.1%
>>>>>>> Call_Recorder            437        426   -11    +2.5%
>>>>>>> Color_Fill_3D            190        180   -10    +5.3%
>>>>>>> eToro                    512        505    -7    +1.4%
>>>>>>> GroupMe                  281        266   -15    +5.3%
>>>>>>>
>>>>>> Hey Bongkyu,
>>>>>>    I wanted to reach out to see what the current status of this patch
>>>>>> set? I'm seeing other parties trying to work around the loss of the
>>>>>> optimistic spinning functionality since commit 617f3ef95177
>>>>>> ("locking/rwsem: Remove reader optimistic spinning") as well, with
>>>>>> their own custom variants (providing some substantial gains), and
>>>>>> would really like to have a common solution.
>>>>>>
>>>>> I didn't get an reply, so I've been waiting.
>>>>> Could you let me know about their patch?
>>>> I don't have insight/access to any other implementations, but I have
>>>> nudged folks to test your patch and chime in here.
>>>>
>>>> Mostly I just wanted to share that others are also seeing performance
>>>> trouble from the loss of optimistic spinning, so it would be good to
>>>> get some sort of shared solution upstream.
>>>>
>>>> thanks
>>>> -john
>>>>
>> When this patch series was originally posted last year, we gave some
>> comments and suggestion on how to improve it as well as request for
>> more information on certain area. We were expecting a v2 with the
>> suggested changes, but we never got one and so it just fell off the
>> cliff.
>>
>> Please send a v2 with the requested change and we can continue our
>> discussion.
> 
> The major reason that reader optimistic spinning was taken out is
> because of reader fragmentation especially now that we essentially wake
> up all the readers all at once when it is reader's turn to take the read
> lock. I do admit I am a bit biased toward systems with large number of
> CPU cores. On smaller systems with just a few CPU cores, reader
> optimistic spinning may help performance. So one idea that I have is
> that one of the command line option values is an auto mode (beside on
> and off) that reader optimistic spinning is enabled for, say, <= 8 CPUs,
> but disabled with more CPUs.
> 
> Anyway, this is just one of my ideas.
> 
> Cheers,
> Longman
> 
> 

Hi Longman,

Including your idea, I will reconsider and resend patch.

Thanks,
Bongkyu