From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x234.google.com (mail-lj1-x234.google.com [IPv6:2a00:1450:4864:20::234]) by sourceware.org (Postfix) with ESMTPS id EBF93385840C for ; Mon, 13 Dec 2021 13:42:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EBF93385840C Received: by mail-lj1-x234.google.com with SMTP id b19so21933457ljr.12 for ; Mon, 13 Dec 2021 05:42:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=KvRg+cYN+XrtFTbjaSswvy/0SuQCl0ceI1tBprzOGag=; b=eRivE2DNx0CTWckHJUfs7vpslfmICmzdcoTKpoSdWEA0Sbzt4YZHNutHWx2aTv8u+A 1LwjNCtMTet98cTNIOCyMbbK64CQf5HzeGYEC1dDmCtP4GSGfSEp1omAxAFduVuBcnyq ccBIUyBPhvuwRrV3nRwjIicr1wzDLaBBYSliAWorF/vNAln0VhRkAJl7tfx3oJEgis1L oG2XLzLq7bCrN7ekWlZ2DBDSeZM66ZJpad9dnW9riYibt1DlnEw5w8MJ92zmmQCJ9hOt jr+dYknueUAmCrGdDBD2nfGK9YWB0WMtvV1sY0LKXOoBSa6982fUID4YEJagMzUv0pyH hWEA== X-Gm-Message-State: AOAM533hTAi6gMvQhn3YeA4egamIO66UOHvXvUdMIwDwV4204M60TVLq gtL6cSntYhTbZr1plQOVOdtAHmbwF6dF X-Google-Smtp-Source: ABdhPJzQnhOYmslJPl1JHQX4iEZ94CouhVpVkoIZ5IQQZIyP+FtUAOgyryQxwvXsIp/d8gBkahuHFA== X-Received: by 2002:a2e:81da:: with SMTP id s26mr29178959ljg.94.1639402975806; Mon, 13 Dec 2021 05:42:55 -0800 (PST) Received: from [192.168.0.135] ([185.30.228.158]) by smtp.gmail.com with ESMTPSA id q24sm1436771lfp.103.2021.12.13.05.42.55 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Dec 2021 05:42:55 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.7\)) Subject: Re: [PATCH] nptl: Effectively skip CAS in spinlock loop From: Maxim Kuvyrkov In-Reply-To: Date: Mon, 13 Dec 2021 16:42:54 +0300 Cc: libc-alpha@sourceware.org Content-Transfer-Encoding: quoted-printable Message-Id: <185F7E1E-A328-4C62-BE90-221402DA69B0@linaro.org> References: <20211205152814.13597-1-6812skiii@gmail.com> <331FA4DD-8EE9-4538-A462-B68A3617E41C@linaro.org> To: Jangwoong Kim <6812skiii@gmail.com> X-Mailer: Apple Mail (2.3608.120.23.2.7) X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Dec 2021 13:42:59 -0000 Hi Jangwoong, I needed a minute to stare at your patch to understand where extra CAS = execution comes from, and I meant adding a comment like the following to = make code easier to understand. + if (LLL_MUTEX_TRYLOCK (mutex) =3D=3D 0) /* Do not be tempted to move this check to the loop = condition. See commit that added this comment for details. */ + break; Feel free to merge your patch as-is, I may be overthinking this. Regards, -- Maxim Kuvyrkov https://www.linaro.org > On 6 Dec 2021, at 21:03, Jangwoong Kim via Libc-alpha = wrote: >=20 > Hi Maxim, >=20 > Did you mean editing the commit message? >=20 > If so, I will resend a patch right away. >=20 > Or, if you meant adding a comment to source code, I'm not clear on = your request. >=20 > The reason do-while loop structure causes an extra CAS is "continue" > expression does not skip the conditional expression do-while loop. >=20 > My patch replaces the do-while loop, so there isn't an extra CAS. >=20 > Thus, adding a comment why the previous code was wrong doesn't seem = proper. >=20 > -- > Thank you. > Jangwoong Kim >=20 > 2021=EB=85=84 12=EC=9B=94 6=EC=9D=BC (=EC=9B=94) =EC=98=A4=ED=9B=84 = 11:26, Maxim Kuvyrkov =EB=8B=98=EC=9D=B4 = =EC=9E=91=EC=84=B1: >>=20 >>=20 >>> On 5 Dec 2021, at 18:28, Jangwoong Kim via Libc-alpha = wrote: >>>=20 >>> The commit: >>> "Add LLL_MUTEX_READ_LOCK [BZ #28537]" >>> SHA1: d672a98a1af106bd68deb15576710cd61363f7a6 >>>=20 >>> introduced LLL_MUTEX_READ_LOCK, to skip CAS in spinlock loop >>> if atmoic load fails. But, "continue" inside of do-while loop >>> does not skip the evaluation of escape expression, thus CAS >>> is not skipped. >>>=20 >>> Replace do-while with while and break if LLL_MUTEX_READ_LOCK fails. >>> --- >>> nptl/pthread_mutex_lock.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>=20 >>> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c >>> index 47b88a6b5b..24936c8d56 100644 >>> --- a/nptl/pthread_mutex_lock.c >>> +++ b/nptl/pthread_mutex_lock.c >>> @@ -138,7 +138,7 @@ PTHREAD_MUTEX_LOCK (pthread_mutex_t *mutex) >>> int cnt =3D 0; >>> int max_cnt =3D MIN (max_adaptive_count (), >>> mutex->__data.__spins * 2 + 10); >>> - do >>> + while (1) >>> { >>> if (cnt++ >=3D max_cnt) >>> { >>> @@ -148,8 +148,9 @@ PTHREAD_MUTEX_LOCK (pthread_mutex_t *mutex) >>> atomic_spin_nop (); >>> if (LLL_MUTEX_READ_LOCK (mutex) !=3D 0) >>> continue; >>> + if (LLL_MUTEX_TRYLOCK (mutex) =3D=3D 0) >>> + break; >>> } >>> - while (LLL_MUTEX_TRYLOCK (mutex) !=3D 0); >>=20 >> Giving this a second look, would you please add a comment before = =E2=80=9Cbreak=E2=80=9D to explain why do-while loop structure would = cause an extra compare-and-swap? >>=20 >> -- >> Maxim Kuvyrkov >> https://www.linaro.org >>=20 >>>=20 >>> mutex->__data.__spins +=3D (cnt - mutex->__data.__spins) / 8; >>> } >>> -- >>> 2.25.1 >>>=20 >>=20