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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 7EC1BC001DE for ; Sun, 23 Jul 2023 01:57:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=uAbns4snPDnbSZUCvrMAoqZEuAEdGmVRbozqZ2mt9zA=; b=N2e5b97mS2DD5b kCPmA/j2D65sUZBsq+5oXfxfiuEvOhcPbALVxug6e9tCnoK9pjWNRLn6hZ9UpAbNz8ZC/GHHBy5yf xz+ZqvnlC7JQH/YBVF5RU1BarnN7OSvNkAjbaM0YPTRxECWBvh95pYMraTiw9s9n8LqOLfa9+72it j/FUERXp5Q4kZ6AYvHDBtYhx/fa5HLza/9HAD6Ix2XTMUahoGqlzXXkFf8RamW63mxOxfTV5ehezu vS5rp6f5ejwca68PIc9DqisvpQLBrFEWFmVGQ2UIeRcglVoIgDYzuAsVmHvt2tBqwgVwMkkhnkRCT wNx29LdMrEnMXPzwk8nw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qNOLo-0002ns-1P; Sun, 23 Jul 2023 01:57:36 +0000 Received: from mail-pl1-x629.google.com ([2607:f8b0:4864:20::629]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qNOLi-0002gW-2L for linux-arm-kernel@lists.infradead.org; Sun, 23 Jul 2023 01:57:33 +0000 Received: by mail-pl1-x629.google.com with SMTP id d9443c01a7336-1b9ecf0cb4cso18408585ad.2 for ; Sat, 22 Jul 2023 18:57:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690077447; x=1690682247; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=6OJA1szbHJT2dTZkEYp229VJOjQYpcsaqtJfcTXaGHw=; b=SPvSXlRUArMMM7H0n77+u0MIxjCm0vQfLvSGUieskKSANsRFdURbBL3lB76raHnwu+ fHmbkGEKmVewVfwXRvSQUtPZ2nzR+esLoi0N50P1mq+gaF/Gk6tgCwrw25HGIYqFY4oB 9VIu3CMdsQ1WadxBHWCqqt2hyBChrHr4SSpiaD2ooZ698Ia6xwJb4IhEVUwK/vMq4lG9 kX7FWhibBMldmFN2KHl2sDJaex22YuZhL8oCJ9apwSD0qZ3Ym1a5+ULFbgGBBBduhMSe G2GEeQtMts1weJzJmTHApMoKE1Eg5w1RJzwgmPAfD2ZtrB1VZStOK7WM49YG+cDWhn6/ uYLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690077447; x=1690682247; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=6OJA1szbHJT2dTZkEYp229VJOjQYpcsaqtJfcTXaGHw=; b=iH+pPCR35Rp//mNEPdgl6at2rOQ0iImzOO1bmUDWXY0g4arSeXnSf3IqXA3wH7VKDl 1ptHj5C8W07Q63Hx9xplDkUqBfpJVsV6U2iIdFcSIMIIVR1+tJCJIQ+tbmGMb9CuGYFj 4pbnkrP/Izg6FQpmgqursXUummavbgNOjXR6Xkfu6/ALWA5YNAu3JOvM17meatMfaW5G TWAhih9z6fU/EAo41oxQG4neY4cSrSMvhvxI1aVmpkih9EWvKiQPyF2OYa8epL7gPnpW PMzzZhUUqPDAPG06SEFLJhR36mdNR3c3mrGjvjd75NoQlV739Pk5NNkI0RNIKRxTlfJc IjDQ== X-Gm-Message-State: ABy/qLbgzW3YvbqP0hlAtZZm2zLdD9ipgmRpFdTVHvJ6Z/RdHDvgCxwi pfMEvzOvl6RXxIbwcy6lW2M= X-Google-Smtp-Source: APBJJlGhAzGcE+2IOC+BjPU/5Vwb4rjWM8XqjTX5uJY2OU0MetPGTmcbrM2iQkJFc8VrJyG5hCZqcQ== X-Received: by 2002:a17:902:6ac6:b0:1bb:6875:5a73 with SMTP id i6-20020a1709026ac600b001bb68755a73mr4371328plt.2.1690077446732; Sat, 22 Jul 2023 18:57:26 -0700 (PDT) Received: from localhost ([216.228.127.128]) by smtp.gmail.com with ESMTPSA id jk4-20020a170903330400b001b8b6a19bd6sm6048646plb.63.2023.07.22.18.57.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Jul 2023 18:57:26 -0700 (PDT) Date: Sat, 22 Jul 2023 18:57:23 -0700 From: Yury Norov To: Alexander Potapenko Cc: catalin.marinas@arm.com, will@kernel.org, pcc@google.com, andreyknvl@gmail.com, andriy.shevchenko@linux.intel.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, eugenis@google.com, syednwaris@gmail.com, william.gray@linaro.org, Arnd Bergmann Subject: Re: [PATCH v4 1/5] lib/bitmap: add bitmap_{set,get}_value() Message-ID: References: <20230720173956.3674987-1-glider@google.com> <20230720173956.3674987-2-glider@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230720173956.3674987-2-glider@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230722_185730_793597_EB36E441 X-CRM114-Status: GOOD ( 23.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jul 20, 2023 at 07:39:52PM +0200, Alexander Potapenko wrote: > +/** > + * bitmap_write - write n-bit value within a memory region > + * @map: address to the bitmap memory region > + * @value: value of nbits > + * @start: bit offset of the n-bit value > + * @nbits: size of value in bits, up to BITS_PER_LONG > + */ > +static inline void bitmap_write(unsigned long *map, > + unsigned long value, > + unsigned long start, unsigned long nbits) > +{ > + size_t index = BIT_WORD(start); > + unsigned long offset = start % BITS_PER_LONG; > + unsigned long space = BITS_PER_LONG - offset; > + > + if (unlikely(!nbits)) > + return; > + value &= GENMASK(nbits - 1, 0); Strictly speaking, a 'value' shouldn't contain set bits beyond nbits because otherwise it's an out-of-bonds type of error. This is kind of gray zone to me, because it's a caller's responsibility to provide correct input. But on the other hand, it would be a great headache debugging corrupted bitmaps. Now that we've got a single user of the bitmap_write, and even more, it's wrapped with a helper, I think it would be reasonable to trim a 'value' in the helper, if needed. Anyways, the comment must warn about that loudly... > + if (space >= nbits) { > + map[index] &= ~(GENMASK(nbits - 1, 0) << offset); 'GENMASK(nbits - 1, 0) << offset' looks really silly. > + map[index] |= value << offset; Here you commit 2 reads and 2 writes for a word instead of one. > + return; > + } > + map[index] &= ~BITMAP_FIRST_WORD_MASK(start); ~FIRST_WORD is the same as LAST WORD. I tried to replace, and it saves ~25 bytes of .text on x86_64. > + map[index] |= value << offset; > + map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits); > + map[index + 1] |= (value >> space); > +} With all that I think the implementation should look something like this: unsigned long w, mask; if (unlikely(nbits == 0)) return 0; map += BIT_WORD(start); start %= BITS_PER_LONG; end = start + nbits - 1; w = *map & (end < BITS_PER_LONG ? ~GENMASK(end, start) : BITMAP_LAST_WORD_MASK(start)); *map = w | (value << start); if (end < BITS_PER_LONG) return; w = *++map & BITMAP_FIRST_WORD_MASK(end); *map = w | (value >> BITS_PER_LONG * 2 - end); It's not tested, except the /s/~FIRST_WORD/LAST_WORD/ part, but I expect it should be more efficient. Alexander, can you please try the above and compare? In previous iteration, I asked you to share disassembly listings for the functions. Can you please do that now? Regarding the rest of the series: - I still see Evgenii's name in mtecomp.c, and EA0 references; - git-am throws warning about trailing line; - checkpatch warns 7 times; Can you fix all the above before sending the new version? Have you tested generic part against BE32, BE64 and LE32 architectures; and arch part against BE64? If not, please do. You're mentioning that the compression ratio is 2 to 20x. Can you share the absolute numbers? If it's 1k vs 2k, I think most people just don't care... Can you share the code that you used to measure the compression ratio? Would it make sense to export the numbers via sysfs? Thanks, Yury _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel