From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751577AbdB0PCh convert rfc822-to-8bit (ORCPT ); Mon, 27 Feb 2017 10:02:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59996 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750934AbdB0PCg (ORCPT ); Mon, 27 Feb 2017 10:02:36 -0500 Subject: Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present To: Davidlohr Bueso References: <1487786634-22641-1-git-send-email-longman@redhat.com> <1487786634-22641-2-git-send-email-longman@redhat.com> <20170226184924.GE5126@linux-80c1.suse> Cc: Ingo Molnar , Peter Zijlstra , linux-kernel@vger.kernel.org From: Waiman Long Organization: Red Hat Message-ID: <438e0ad2-c263-679d-d369-be9750b2c646@redhat.com> Date: Mon, 27 Feb 2017 10:02:36 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170226184924.GE5126@linux-80c1.suse> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8BIT X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 27 Feb 2017 15:02:37 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/26/2017 01:49 PM, Davidlohr Bueso wrote: > On Wed, 22 Feb 2017, Waiman Long wrote: > >> We can safely check the wait_list to see if waiters are present without >> lock when there are spinners to fall back on in case we miss a waiter. >> The advantage is that we can save a pair of spin_lock/unlock calls >> when the wait_list is empty. This translates to a reduction in latency >> and hence slightly better performance. > > This benefit is only seen in (rare) situations where there are only > writers with short hold times, no? I don't really have any objection > as I doubt the additional load will have any impact on the common case, > but it would still be nice to have more data for other benchmarks where > the lock is at least shared at times -- ie: a good thing to measure is > also fault, mmap related benchmarks. If a up_write() or up_read() coincides with a writer attempting to lock (down_write). The unlocker may go into the wake_rwsem path even if on one is on the wait queue. In this case, this patch can save an unneeded spin_lock/unlock. This was what happened in the microbenchmark that I used. The additional load shouldn't have any noticeable performance impact as the wait_list need to be read sooner or later anyway. BTW, can you suggest a good benchmark for testing fault, mmap related code paths? >> + /* >> + * Normally checking wait_list without wait_lock isn't safe >> + * as we may miss an incoming waiter. With spinners present, >> + * however, we have someone to fall back on in case that >> + * happens. This can save a pair of spin_lock/unlock calls >> + * when there is no waiter. >> + */ > > I would drop the last part regarding saving the spin_lock, it should be > evident from the code. I will do that. Cheers, Longman