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 1BB72E810DA for ; Wed, 27 Sep 2023 12:11:08 +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=f279n5o/eeg5iGzuJG8sNwh8I4vCOvusYz6Lyc5DmZ0=; b=FYyCw1n9AwNxt1 4J9JdPpyeLB4jvdryDAK6oNOGVysALPaMN/qNpclk6z/bGjTdee7f2Vcn/s3K5sOFKl9710IYf8XW B3dFOPZFuQvEsNrJx2rVYLU+AcrkoLdjm0yWk3XrjzcLXUrY7Bz2dNPBjFzjkCOWe+TF8Kbac9A5b lb4E1EwKFeatfMxm44pjCzz0wtPI/bShgHUODkpTSN9+2a4xC+0aiNpVNtge70kaAznZmsPnzHl+M A0ONHhihBlXE6N06fNcqxGSGVTulx76XV4Sd/tQIDvMMwRobVHJVX+hZgmh8O55RfaPD5tpQmWVKo XdiTLu6hdqWb0LrywYUg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qlTNI-000r3Y-1U; Wed, 27 Sep 2023 12:10:40 +0000 Received: from mgamail.intel.com ([134.134.136.20]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qlTN8-000r1E-0X for linux-arm-kernel@lists.infradead.org; Wed, 27 Sep 2023 12:10:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695816630; x=1727352630; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=qpBb1Dvmlu54LR+5l2f0lLgg+l2XFidwR6LH6371XtU=; b=ddQtFxwwEzMXR3BfgroFWquroJ4N51Vi1ByQ8sbg/7SlQQG743RbmZLI B9rULFNOVJOzfQIeVaksoc27VM0pREjmgklR//5FIocxuC1+2RzEGkKz+ GOC6tgVGyDDOYYkKNARRRp8DF8aqCyY8sFpcfwH0X7Gqr0/+7eMgwBSbz RpAetda5pzSEcYriZe3hx2UGzl17tRWCSDi9yIOgEEfJdb0YZY1snbtsP RN7UW+Brtg3ppE4BI4hKnKj5teDPz1rQmboTI3Cz/cr7u13Jh8wQf5zbT WhtxmmjmhUR4F9sSvq6qsVffIk+x054cdGsUpMdsFsqZEIPb2LuEaYzGz g==; X-IronPort-AV: E=McAfee;i="6600,9927,10845"; a="372150464" X-IronPort-AV: E=Sophos;i="6.03,181,1694761200"; d="scan'208";a="372150464" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2023 05:10:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10845"; a="892580517" X-IronPort-AV: E=Sophos;i="6.03,181,1694761200"; d="scan'208";a="892580517" Received: from smile.fi.intel.com ([10.237.72.54]) by fmsmga001.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Sep 2023 05:09:16 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.97-RC0) (envelope-from ) id 1qlTFT-00000000sRB-0Bl3; Wed, 27 Sep 2023 15:02:35 +0300 Date: Wed, 27 Sep 2023 15:02:34 +0300 From: Andy Shevchenko To: Yury Norov Cc: Linus Walleij , Bartosz Golaszewski , linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Shubhrajyoti Datta , Srinivas Neeli , Michal Simek , Bartosz Golaszewski , Rasmus Villemoes , Marek =?iso-8859-1?Q?Beh=FAn?= Subject: Re: [PATCH v1 2/5] lib/bitmap: Introduce bitmap_scatter() and bitmap_gather() helpers Message-ID: References: <20230926052007.3917389-1-andriy.shevchenko@linux.intel.com> <20230926052007.3917389-3-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230927_051030_247888_604C5D73 X-CRM114-Status: GOOD ( 38.35 ) 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 Tue, Sep 26, 2023 at 05:25:13PM -0700, Yury Norov wrote: > On Tue, Sep 26, 2023 at 08:20:04AM +0300, Andy Shevchenko wrote: > > These helpers are the optimized versions of the bitmap_remap() > > where one of the bitmaps (source or destination) is of sequential bits. > > If so, can you add a test that makes sure that new API is consistent > with the old bitmap_remap? And also provide numbers how well are they > optimized, comparing to bitmap_remap. It's impossible. bitmap_remap() is universal, these APIs only for the specific domain. > > See more in the kernel documentation of the helpers. > > I grepped the whole kernel, not only Documentation directory, and found > nothing... It's added in this patch in the format of kernel doc. ... > > + * Returns: the weight of the @mask. > > Returning a weight of the mask is somewhat non-trivial... To me it > would be logical to return a weight of destination, for example... > But I see that in the following patch you're using the returned value. > Maybe add a few words to advocate that? I'll look into it again, maybe dst would work as well, I don't remember why I have chosen mask. Maybe because it's invariant here, dunno. ... > > + int n = 0; > > Is n signed for purpose? I think it should be consistent with > return value. OK. No purpose there. Perhaps it's a leftover from the first experiments on the implementation of these APIs. ... > > + * Example: > > + * If @src bitmap = 0x0302, with @mask = 0x1313, @dst will be 0x001a. > > Not sure about others, but to me hex representation is quite useless, > moreover it's followed by binary one. Somebody is better at hex, somebody at binary one, I would leave both. > > + * Or in binary form > > + * @src @mask @dst > > + * 0000001100000010 0001001100010011 0000000000011010 > > + * > > + * (Bits 0, 1, 4, 8, 9, 12 are copied to the bits 0, 1, 2, 3, 4, 5) > > + * > > + * Returns: the weight of the @mask. > > + */ > > It looks like those are designed complement to each other. Is that > true? If so, can you make your example showing that > scatter -> gather -> scatter > would restore the original bitmap? It looks like you stopped reading documentation somewhere on the middle. The two APIs are documented with the same example which makes it clear that they are data-loss transformations. Do you need something like this to be added (in both documentations): The bitmap_scatter(), when executed over the @dst bitmap, will restore the @src one if the @mask is kept the same, see the example in the function description. ? > If I'm wrong, can you please underline that they are not complement, > and why? No, you are not. ... > I feel like they should reside in header, because they are quite a small > functions indeed, and they would benefit from compile-time optimizations > without bloating the kernel. > > Moreover, you are using them in patch #3 on 64-bit bitmaps, which > would benefit from small_const_nbits() optimization. I can move them into header. ... > > + DECLARE_BITMAP(bmap, 1024); > Can you make it 1000? That way we'll test non-aligned case. Sure. But it's not related to the patch. It will test bitmap_weight() and not the new APIs, so, whatever is bigger 64 will suffice the purpose and won't anyhow affect the newly added APIs. ... > Would be interesting to compare bitmap scatter/gather performance > against bitmap_remap. Do you have a code in mind? I can incorporate it. Again, you should understand that it's only applicable to the certain cases, otherwise it makes no sense (it's like comparing performance of ffs() on a single word against find_first_bit() on the arbitrary amount of words). -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel