From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19B14C433DB for ; Mon, 28 Dec 2020 12:12:50 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BB10F2074D for ; Mon, 28 Dec 2020 12:12:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BB10F2074D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MjWBXJk3VQTDm9hS/4wxPeYhmiJxJfyKuRXcyNyGrPs=; b=qWcrmVEhC9WUHtU8OImqDhOME eVN+JurRU1duK3ez9AM5ppwbyCoVuGLwt+91y+/S0nvrG0OscXk06pp7G6Gfh13nUZQ9Lfr0AF1v+ ZjopCwyKrgUjXa+/yWgDnHnh4fLTZU1fM2niLP8P7Ida4X8Hsi8cdtR5gX2by7J+sf509gAYrysFa JwLFa2g597E0ARbqNNYPTF9PjHHoEZR19K2u39MGeRyLF2rXs4tJjGOoTTizhJ5VJITuGmwgTWmlb t7DwYM6NlA2JF75zBTYtGOM8LSBlV5j05Y6LPjOWf5OCy+Y50jE6shwFQvDkXfZjPoIlNWQpZ2IAK k8ba1MWFg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ktrMd-0008Tj-FT; Mon, 28 Dec 2020 12:11:03 +0000 Received: from mail-qk1-x735.google.com ([2607:f8b0:4864:20::735]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ktrMa-0008SS-Mg for linux-arm-kernel@lists.infradead.org; Mon, 28 Dec 2020 12:11:01 +0000 Received: by mail-qk1-x735.google.com with SMTP id f26so8639410qka.0 for ; Mon, 28 Dec 2020 04:11:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=n3AT6TDkcYOgoqXitcYuftfnSMBgPOjVyKdhjDG6q98=; b=J1zm2pYkcwwgV3idpyV51/hd6OLd3cUy1dOzgyZLHS+FqU6Zp9Z7IP4KCwC0pyefgF +F8Ua+oVw5ubQlnsCWMOUEdVg6txT62/4DgMs0TyjA/sRNmq7risll2xB6UOF5H5/ICW eQ37vjgb0rObWr5xKSgO+8j/aH57V6hjKkMOPxfseNLl0yNnD89Iq5esifqFqeTKYTal khpreON5T50lGDPsmPqHQ+WhIDjaT36oOqAiGqnQEA+Qer5DzUCHnqUWGdw1NjWjQ1ZA 2dF4FIP9TPbwmBZIGP/syv560pQsbvZcOgPFPkEPjscLxqMR4cKfldLuZzlRllycsLdH XI4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=n3AT6TDkcYOgoqXitcYuftfnSMBgPOjVyKdhjDG6q98=; b=GEqUyhOdCgyc8mxndZMn5OIfFP22VbIVve+2JTWWRLD+xvBNgSxgG4eGY8cIxNvyxY 7AIvMrZj77cwGnq48xPbaHHIChfKTRpTT3ZD4EaTqWN7Y7x7o8Lmoa8bqzcodRgVddQm K5xGkZ5L1sDaSKLH8Dk2WTgBiJoWdmCQxTPS40VhE6LtY7canALW6SueobhqDq0M2GlV A5U8Ra/PY32gj8E+m6H767f3exbB+2YpU2zcnclretPdZC0goO0oSCBzFVelcJhOESHO ugl326G14Xg4gNBLuCBkOKM0RO7/qWQrFFLAfDg8ofvQN1JeGSIIq6c6nMj4PjB42HsD wyJg== X-Gm-Message-State: AOAM530VSoLqBjDfAKm62CLuXVuzUWj/zrqPV9g9MwYQVjle+3r+7EA+ rFCyNt5qPe2iU0ZHvgH29GB/ZYIyePv34Q== X-Google-Smtp-Source: ABdhPJwxg0ALuDYcBPG8okEWFoPmRVsrFJu61+U6tOu14LDoApF+Zy919bFYomUc2jGdbbUFdQA67w== X-Received: by 2002:a37:73c6:: with SMTP id o189mr44162144qkc.169.1609157458688; Mon, 28 Dec 2020 04:10:58 -0800 (PST) Received: from shinobu (072-189-064-225.res.spectrum.com. [72.189.64.225]) by smtp.gmail.com with ESMTPSA id z15sm23611656qkz.103.2020.12.28.04.10.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Dec 2020 04:10:57 -0800 (PST) Date: Mon, 28 Dec 2020 07:10:55 -0500 From: William Breathitt Gray To: Arnd Bergmann Subject: Re: [PATCH 1/5] clump_bits: Introduce the for_each_set_clump macro Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201228_071100_850099_3A7CB942 X-CRM114-Status: GOOD ( 34.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch , Amit Kucheria , Arnd Bergmann , Masahiro Yamada , Linus Walleij , Daniel Lezcano , Michal Simek , "linux-kernel@vger.kernel.org" , Bartosz Golaszewski , Robert Richter , "open list:GPIO SUBSYSTEM" , Linux PM list , Andrew Morton , Andy Shevchenko , Syed Nayyar Waris , Zhang Rui , Linux ARM Content-Type: multipart/mixed; boundary="===============0738761378832138238==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============0738761378832138238== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="jvtA3Yi9DvLrtiYs" Content-Disposition: inline --jvtA3Yi9DvLrtiYs Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Dec 27, 2020 at 11:03:06PM +0100, Arnd Bergmann wrote: > On Sat, Dec 26, 2020 at 7:42 AM Syed Nayyar Waris = wrote: > > > > This macro iterates for each group of bits (clump) with set bits, > > within a bitmap memory region. For each iteration, "start" is set to > > the bit offset of the found clump, while the respective clump value is > > stored to the location pointed by "clump". Additionally, the > > bitmap_get_value() and bitmap_set_value() functions are introduced to > > respectively get and set a value of n-bits in a bitmap memory region. > > The n-bits can have any size from 1 to BITS_PER_LONG. size less > > than 1 or more than BITS_PER_LONG causes undefined behaviour. > > Moreover, during setting value of n-bit in bitmap, if a situation arise > > that the width of next n-bit is exceeding the word boundary, then it > > will divide itself such that some portion of it is stored in that word, > > while the remaining portion is stored in the next higher word. Similar > > situation occurs while retrieving the value from bitmap. > > > > GCC gives warning in bitmap_set_value(): https://godbolt.org/z/rjx34r > > Add explicit check to see if the value being written into the bitmap > > does not fall outside the bitmap. > > The situation that it is falling outside would never be possible in the > > code because the boundaries are required to be correct before the > > function is called. The responsibility is on the caller for ensuring the > > boundaries are correct. > > The code change is simply to silence the GCC warning messages > > because GCC is not aware that the boundaries have already been checked. > > As such, we're better off using __builtin_unreachable() here because we > > can avoid the latency of the conditional check entirely. >=20 > Didn't the __builtin_unreachable() end up leading to an objtool > warning about incorrect stack frames for the code path that leads > into the undefined behavior? I thought I saw a message from the 0day > build bot about that and didn't expect to see it again after that. >=20 > Can you actually measure any performance difference compared > to BUG_ON() that avoids the undefined behavior? Practically > all CPUs from the past 20 years have branch predictors that should > completely hide measurable overhead from this. >=20 > Arnd When I initially recommended using __builtin_unreachable(), I was anticipating the use of bitmap_set_value() in kernel at large -- so the possible performance hit from a conditional check was a concern for me. However, now that we're restricting the scope of bitmap_set_value() to only the GPIO subsystem, such optimization is no longer a major concern I feel: gpio-xilinx is the only driver utilizing bitmap_set_value() -- and we know it won't be called in a loop -- so whatever hypothetical performance hit there might be is inconsequential in the end. Instead, we should focus on code clarity now. I believe it makes sense given the new scope of this function to revert back to the earlier suggestion of passing in and checking the boundary explicitly, and to remove the __builtin_unreachable() call for now. If bitmap_set_value() becomes available to the rest of the kernel in the future, we can reconsider whether or not to use __builtin_unreachable(). William Breathitt Gray --jvtA3Yi9DvLrtiYs Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAl/py08ACgkQhvpINdm7 VJJdtA//cR1YiOviFw5k9cxcAoNFUWhX1LfFPSZzBwGgXnPNVUeQDxCYt9Zo7m48 bEcIumIHZBm0W8tySv4BlQBBSTEiiBFOWKfNHRpLJES2oVBXaYJppe6nMMjHcb0v 5gZKxeFTC+51ZZKw238Csned7xNCUDiYDOjwvFCWuccF0tadOWWKNLqWYpl2LlwX VcOe64W7/N7Wd2X7zCg/6jwIEu5RNR7I1oSt5DMNCraQjwBBRpKxoTRwb5sT/60x xWQCJYA7qg3jseisk0n2xyAAh3nA8JWCDx/XF7qnt0+vz37Q7sia3mJKZiL3j2Ad yTgBDbddWPn2f4qEMpFijgfRRFzJ1jKHwwWG4n086aeC3Ql608QEMU4arJInAzgK 3NxrdoY01mKqNye7dSac9JyvGu1KZLc6QPiZ5//sxdYG91yEnaqf9xpHgft8wobp 5/C2dMFbZjktcFNuepxgLjPNuB+J202lEifiwAMI2sL8h9hCfyWi/U8O9WDuFosY 9KChwHyjV4eG2GMqnIhxAG0bA94xuJNp8avH3+8CAB5mc+05RFw3nwS4fFG4YsbS a4XY1+wtAkyI2aqpoRa606RlaVvoBRzMWt0LEc2+Fy6Ab/98svG26coD8GcZ/RUN /ElXNOM7mq1Jl2utzF5v86oTEtK0K1cvz+XDrRqYsYObM1yC+ts= =uoh3 -----END PGP SIGNATURE----- --jvtA3Yi9DvLrtiYs-- --===============0738761378832138238== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============0738761378832138238==--