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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 F2AC0C432C3 for ; Wed, 27 Nov 2019 19:06:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D734A20835 for ; Wed, 27 Nov 2019 19:06:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727105AbfK0TGV (ORCPT ); Wed, 27 Nov 2019 14:06:21 -0500 Received: from foss.arm.com ([217.140.110.172]:52062 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727010AbfK0TGV (ORCPT ); Wed, 27 Nov 2019 14:06:21 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 04C6F31B; Wed, 27 Nov 2019 11:06:20 -0800 (PST) Received: from [10.1.196.37] (e121345-lin.cambridge.arm.com [10.1.196.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D3B673F6C4; Wed, 27 Nov 2019 11:06:14 -0800 (PST) Subject: Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions To: Nicolas Saenz Julienne , Leon Romanovsky Cc: andrew.murray@arm.com, maz@kernel.org, linux-kernel@vger.kernel.org, Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Tariq Toukan , Rob Herring , Frank Rowand , Shawn Lin , Heiko Stuebner , Christoph Hellwig , Marek Szyprowski , james.quinlan@broadcom.com, mbrugger@suse.com, f.fainelli@gmail.com, phil@raspberrypi.org, wahrenst@gmx.net, jeremy.linton@arm.com, linux-pci@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, "Rafael J. Wysocki" , Len Brown , "David S. Miller" , Bjorn Helgaas , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org, linux-rdma@vger.kernel.org, devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org, iommu@lists.linux-foundation.org References: <20191126091946.7970-1-nsaenzjulienne@suse.de> <20191126091946.7970-2-nsaenzjulienne@suse.de> <20191126125137.GA10331@unreal> <6e0b9079-9efd-2884-26d1-3db2d622079d@arm.com> From: Robin Murphy Message-ID: Date: Wed, 27 Nov 2019 19:06:12 +0000 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote: > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote: >> On 26/11/2019 12:51 pm, Leon Romanovsky wrote: >>> On Tue, Nov 26, 2019 at 10:19:39AM +0100, Nicolas Saenz Julienne wrote: >>>> Some users need to make sure their rounding function accepts and returns >>>> 64bit long variables regardless of the architecture. Sadly >>>> roundup/rounddown_pow_two() takes and returns unsigned longs. Create a >>>> new generic 64bit variant of the function and cleanup rougue custom >>>> implementations. >>> >>> Is it possible to create general roundup/rounddown_pow_two() which will >>> work correctly for any type of variables, instead of creating special >>> variant for every type? >> >> In fact, that is sort of the case already - roundup_pow_of_two() itself >> wraps ilog2() such that the constant case *is* type-independent. And >> since ilog2() handles non-constant values anyway, might it be reasonable >> to just take the strongly-typed __roundup_pow_of_two() helper out of the >> loop as below? >> >> Robin >> > > That looks way better that's for sure. Some questions. > >> ----->8----- >> diff --git a/include/linux/log2.h b/include/linux/log2.h >> index 83a4a3ca3e8a..e825f8a6e8b5 100644 >> --- a/include/linux/log2.h >> +++ b/include/linux/log2.h >> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> */ >> #define roundup_pow_of_two(n) \ >> ( \ >> - __builtin_constant_p(n) ? ( \ >> - (n == 1) ? 1 : \ >> - (1UL << (ilog2((n) - 1) + 1)) \ >> - ) : \ >> - __roundup_pow_of_two(n) \ >> + (__builtin_constant_p(n) && (n == 1)) ? \ >> + 1 : (1UL << (ilog2((n) - 1) + 1)) \ > > Then here you'd have to use ULL instead of UL, right? I want my 64bit value > everywhere regardless of the CPU arch. The downside is that would affect > performance to some extent (i.e. returning a 64bit value where you used to have > a 32bit one)? True, although it's possible that 1ULL might result in the same codegen if the compiler can see that the result is immediately truncated back to long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, however ugly. Either way, this diff was only an illustration rather than a concrete proposal, but it might be an interesting diversion to investigate. On that note, though, you should probably be using ULL in your current patch too. > Also, what about callers to this function on platforms with 32bit 'unsigned > longs' that happen to input a 64bit value into this. IIUC we'd have a change of > behaviour. Indeed, although the change in such a case would be "start getting the expected value instead of nonsense", so it might very well be welcome ;) Robin. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Date: Wed, 27 Nov 2019 19:06:12 +0000 Message-ID: References: <20191126091946.7970-1-nsaenzjulienne@suse.de> <20191126091946.7970-2-nsaenzjulienne@suse.de> <20191126125137.GA10331@unreal> <6e0b9079-9efd-2884-26d1-3db2d622079d@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-GB Sender: linux-kernel-owner@vger.kernel.org To: Nicolas Saenz Julienne , Leon Romanovsky Cc: andrew.murray@arm.com, maz@kernel.org, linux-kernel@vger.kernel.org, Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Tariq Toukan , Rob Herring , Frank Rowand , Shawn Lin , Heiko Stuebner , Christoph Hellwig , Marek Szyprowski , james.quinlan@broadcom.com, mbrugger@suse.com, f.fainelli@gmail.com, phil@raspberrypi.org, wahrenst@gmx.net, jeremy.linton@arm.com, linux-pci@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, "Rafael J. Wysocki" , Len Brown , "David S. Miller" List-Id: linux-rockchip.vger.kernel.org On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote: > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote: >> On 26/11/2019 12:51 pm, Leon Romanovsky wrote: >>> On Tue, Nov 26, 2019 at 10:19:39AM +0100, Nicolas Saenz Julienne wrote: >>>> Some users need to make sure their rounding function accepts and returns >>>> 64bit long variables regardless of the architecture. Sadly >>>> roundup/rounddown_pow_two() takes and returns unsigned longs. Create a >>>> new generic 64bit variant of the function and cleanup rougue custom >>>> implementations. >>> >>> Is it possible to create general roundup/rounddown_pow_two() which will >>> work correctly for any type of variables, instead of creating special >>> variant for every type? >> >> In fact, that is sort of the case already - roundup_pow_of_two() itself >> wraps ilog2() such that the constant case *is* type-independent. And >> since ilog2() handles non-constant values anyway, might it be reasonable >> to just take the strongly-typed __roundup_pow_of_two() helper out of the >> loop as below? >> >> Robin >> > > That looks way better that's for sure. Some questions. > >> ----->8----- >> diff --git a/include/linux/log2.h b/include/linux/log2.h >> index 83a4a3ca3e8a..e825f8a6e8b5 100644 >> --- a/include/linux/log2.h >> +++ b/include/linux/log2.h >> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> */ >> #define roundup_pow_of_two(n) \ >> ( \ >> - __builtin_constant_p(n) ? ( \ >> - (n == 1) ? 1 : \ >> - (1UL << (ilog2((n) - 1) + 1)) \ >> - ) : \ >> - __roundup_pow_of_two(n) \ >> + (__builtin_constant_p(n) && (n == 1)) ? \ >> + 1 : (1UL << (ilog2((n) - 1) + 1)) \ > > Then here you'd have to use ULL instead of UL, right? I want my 64bit value > everywhere regardless of the CPU arch. The downside is that would affect > performance to some extent (i.e. returning a 64bit value where you used to have > a 32bit one)? True, although it's possible that 1ULL might result in the same codegen if the compiler can see that the result is immediately truncated back to long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, however ugly. Either way, this diff was only an illustration rather than a concrete proposal, but it might be an interesting diversion to investigate. On that note, though, you should probably be using ULL in your current patch too. > Also, what about callers to this function on platforms with 32bit 'unsigned > longs' that happen to input a 64bit value into this. IIUC we'd have a change of > behaviour. Indeed, although the change in such a case would be "start getting the expected value instead of nonsense", so it might very well be welcome ;) Robin. 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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable 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 54707C432C3 for ; Wed, 27 Nov 2019 19:06:25 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 275A520835 for ; Wed, 27 Nov 2019 19:06:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 275A520835 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id F2EDC20412; Wed, 27 Nov 2019 19:06:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qwgQQASlhfVx; Wed, 27 Nov 2019 19:06:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 99FBB20023; Wed, 27 Nov 2019 19:06:23 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 826F2C1DDA; Wed, 27 Nov 2019 19:06:23 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 979ACC0881 for ; Wed, 27 Nov 2019 19:06:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 84C59861C9 for ; Wed, 27 Nov 2019 19:06:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JpdZHoO4NKMd for ; Wed, 27 Nov 2019 19:06:20 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by fraxinus.osuosl.org (Postfix) with ESMTP id A272B85A58 for ; Wed, 27 Nov 2019 19:06:20 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 04C6F31B; Wed, 27 Nov 2019 11:06:20 -0800 (PST) Received: from [10.1.196.37] (e121345-lin.cambridge.arm.com [10.1.196.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D3B673F6C4; Wed, 27 Nov 2019 11:06:14 -0800 (PST) Subject: Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions To: Nicolas Saenz Julienne , Leon Romanovsky References: <20191126091946.7970-1-nsaenzjulienne@suse.de> <20191126091946.7970-2-nsaenzjulienne@suse.de> <20191126125137.GA10331@unreal> <6e0b9079-9efd-2884-26d1-3db2d622079d@arm.com> From: Robin Murphy Message-ID: Date: Wed, 27 Nov 2019 19:06:12 +0000 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB Cc: Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Hanjun Guo , Frank Rowand , Christoph Hellwig , f.fainelli@gmail.com, linux-rockchip@lists.infradead.org, linux-rdma@vger.kernel.org, maz@kernel.org, phil@raspberrypi.org, linux-acpi@vger.kernel.org, Len Brown , devicetree@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, Bjorn Helgaas , linux-arm-kernel@lists.infradead.org, mbrugger@suse.com, netdev@vger.kernel.org, "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, jeremy.linton@arm.com, iommu@lists.linux-foundation.org, Rob Herring , wahrenst@gmx.net, james.quinlan@broadcom.com, Sudeep Holla , "David S. Miller" , Tariq Toukan X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote: > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote: >> On 26/11/2019 12:51 pm, Leon Romanovsky wrote: >>> On Tue, Nov 26, 2019 at 10:19:39AM +0100, Nicolas Saenz Julienne wrote: >>>> Some users need to make sure their rounding function accepts and returns >>>> 64bit long variables regardless of the architecture. Sadly >>>> roundup/rounddown_pow_two() takes and returns unsigned longs. Create a >>>> new generic 64bit variant of the function and cleanup rougue custom >>>> implementations. >>> >>> Is it possible to create general roundup/rounddown_pow_two() which will >>> work correctly for any type of variables, instead of creating special >>> variant for every type? >> >> In fact, that is sort of the case already - roundup_pow_of_two() itself >> wraps ilog2() such that the constant case *is* type-independent. And >> since ilog2() handles non-constant values anyway, might it be reasonable >> to just take the strongly-typed __roundup_pow_of_two() helper out of the >> loop as below? >> >> Robin >> > > That looks way better that's for sure. Some questions. > >> ----->8----- >> diff --git a/include/linux/log2.h b/include/linux/log2.h >> index 83a4a3ca3e8a..e825f8a6e8b5 100644 >> --- a/include/linux/log2.h >> +++ b/include/linux/log2.h >> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> */ >> #define roundup_pow_of_two(n) \ >> ( \ >> - __builtin_constant_p(n) ? ( \ >> - (n == 1) ? 1 : \ >> - (1UL << (ilog2((n) - 1) + 1)) \ >> - ) : \ >> - __roundup_pow_of_two(n) \ >> + (__builtin_constant_p(n) && (n == 1)) ? \ >> + 1 : (1UL << (ilog2((n) - 1) + 1)) \ > > Then here you'd have to use ULL instead of UL, right? I want my 64bit value > everywhere regardless of the CPU arch. The downside is that would affect > performance to some extent (i.e. returning a 64bit value where you used to have > a 32bit one)? True, although it's possible that 1ULL might result in the same codegen if the compiler can see that the result is immediately truncated back to long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, however ugly. Either way, this diff was only an illustration rather than a concrete proposal, but it might be an interesting diversion to investigate. On that note, though, you should probably be using ULL in your current patch too. > Also, what about callers to this function on platforms with 32bit 'unsigned > longs' that happen to input a 64bit value into this. IIUC we'd have a change of > behaviour. Indeed, although the change in such a case would be "start getting the expected value instead of nonsense", so it might very well be welcome ;) Robin. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 3FF74C432C0 for ; Wed, 27 Nov 2019 19:06:28 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 0B9AD2080F for ; Wed, 27 Nov 2019 19:06:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="P51NGA6B" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0B9AD2080F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=dvIgUhVdiqP7zRphBsuxHoMMewetpUfST0BrZd499Tk=; b=P51NGA6Bg0ci0SNtB+Dhooni/ eO9eDKbhv+6Jj1qztFip3DnKdbSx7EljyE3WQK3BrRvfRg4r1lw/NKrExuTBYEitSYYXn3e1WJMNl +bDEWHFjFzIH6ryJ9gK08TJ1ZK39wwyjZlK4Qu2gc4Q5NlpSDefCo56MCoIAteo64S5K3Sbkd+c6C BxleopHWpr7mlga+oJo3+/WihDeXJLRwL9Ik9y1MRqbqyDCQRIpRTuUqgtgwwF6ZJxG0zOWZdGp5e mrrIgwCmyyCIlaie5GgGt5ZxFBJI06KfO/UJ9rFRt8bXZGF5mS+o8u3fJEwqqfna6rGMYH7MuhGjs +bgr+XZ+w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ia2ds-0002UV-Rw; Wed, 27 Nov 2019 19:06:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ia2dp-0002TN-1K; Wed, 27 Nov 2019 19:06:22 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 04C6F31B; Wed, 27 Nov 2019 11:06:20 -0800 (PST) Received: from [10.1.196.37] (e121345-lin.cambridge.arm.com [10.1.196.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D3B673F6C4; Wed, 27 Nov 2019 11:06:14 -0800 (PST) Subject: Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions To: Nicolas Saenz Julienne , Leon Romanovsky References: <20191126091946.7970-1-nsaenzjulienne@suse.de> <20191126091946.7970-2-nsaenzjulienne@suse.de> <20191126125137.GA10331@unreal> <6e0b9079-9efd-2884-26d1-3db2d622079d@arm.com> From: Robin Murphy Message-ID: Date: Wed, 27 Nov 2019 19:06:12 +0000 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191127_110621_169334_9C46EABA X-CRM114-Status: GOOD ( 18.48 ) 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: Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Hanjun Guo , Frank Rowand , Christoph Hellwig , Marek Szyprowski , f.fainelli@gmail.com, linux-rockchip@lists.infradead.org, linux-rdma@vger.kernel.org, maz@kernel.org, phil@raspberrypi.org, linux-acpi@vger.kernel.org, Len Brown , devicetree@vger.kernel.org, Lorenzo Pieralisi , linux-rpi-kernel@lists.infradead.org, Bjorn Helgaas , linux-arm-kernel@lists.infradead.org, mbrugger@suse.com, netdev@vger.kernel.org, "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, jeremy.linton@arm.com, iommu@lists.linux-foundation.org, Rob Herring , wahrenst@gmx.net, james.quinlan@broadcom.com, Sudeep Holla , andrew.murray@arm.com, "David S. Miller" , Tariq Toukan Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote: > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote: >> On 26/11/2019 12:51 pm, Leon Romanovsky wrote: >>> On Tue, Nov 26, 2019 at 10:19:39AM +0100, Nicolas Saenz Julienne wrote: >>>> Some users need to make sure their rounding function accepts and returns >>>> 64bit long variables regardless of the architecture. Sadly >>>> roundup/rounddown_pow_two() takes and returns unsigned longs. Create a >>>> new generic 64bit variant of the function and cleanup rougue custom >>>> implementations. >>> >>> Is it possible to create general roundup/rounddown_pow_two() which will >>> work correctly for any type of variables, instead of creating special >>> variant for every type? >> >> In fact, that is sort of the case already - roundup_pow_of_two() itself >> wraps ilog2() such that the constant case *is* type-independent. And >> since ilog2() handles non-constant values anyway, might it be reasonable >> to just take the strongly-typed __roundup_pow_of_two() helper out of the >> loop as below? >> >> Robin >> > > That looks way better that's for sure. Some questions. > >> ----->8----- >> diff --git a/include/linux/log2.h b/include/linux/log2.h >> index 83a4a3ca3e8a..e825f8a6e8b5 100644 >> --- a/include/linux/log2.h >> +++ b/include/linux/log2.h >> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >> */ >> #define roundup_pow_of_two(n) \ >> ( \ >> - __builtin_constant_p(n) ? ( \ >> - (n == 1) ? 1 : \ >> - (1UL << (ilog2((n) - 1) + 1)) \ >> - ) : \ >> - __roundup_pow_of_two(n) \ >> + (__builtin_constant_p(n) && (n == 1)) ? \ >> + 1 : (1UL << (ilog2((n) - 1) + 1)) \ > > Then here you'd have to use ULL instead of UL, right? I want my 64bit value > everywhere regardless of the CPU arch. The downside is that would affect > performance to some extent (i.e. returning a 64bit value where you used to have > a 32bit one)? True, although it's possible that 1ULL might result in the same codegen if the compiler can see that the result is immediately truncated back to long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, however ugly. Either way, this diff was only an illustration rather than a concrete proposal, but it might be an interesting diversion to investigate. On that note, though, you should probably be using ULL in your current patch too. > Also, what about callers to this function on platforms with 32bit 'unsigned > longs' that happen to input a 64bit value into this. IIUC we'd have a change of > behaviour. Indeed, although the change in such a case would be "start getting the expected value instead of nonsense", so it might very well be welcome ;) Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel