From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f171.google.com (mail-yw1-f171.google.com [209.85.128.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 724F01A702 for ; Mon, 9 Oct 2023 16:31:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XnW/hp8q" Received: by mail-yw1-f171.google.com with SMTP id 00721157ae682-59f4bc88f9fso56313257b3.2 for ; Mon, 09 Oct 2023 09:31:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696869077; x=1697473877; darn=lists.linux.dev; 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=ZwJG0ZcdADOiEPsRD1fp8I+0b5ih4BdS68Cgas4xJ7g=; b=XnW/hp8qfv7uaPs/IJgMe/CDfKHl6WBOgu3GHoKPZoBnp1AshXI3W+szYDo7YA+bgd JIONgFsO/XVpJhpiIwObIUQvNZTRu28EyLBMzv6sAJdF/VoZICdwslpToDUcWB7dSU2B sf6UvZiyZtEMyIPCIakZGSrtW44JqNvw1LRS6LiYSbwq83Xh1dFybfpUzpuBufH0Ko6h mXTpCdR/N3anNHq6ppd+Lyn0KFmam3PsoaHH5ycYQcsjuKq6Jcu9GJ9TE50Rp7un2tp9 YhSV3iSEG/9mpf3yesrqTomWwuw5jIxmD2A9vp0p/m9wcm9WVBV4/IRyJ90EXM69E9y+ s5Fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696869077; x=1697473877; 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=ZwJG0ZcdADOiEPsRD1fp8I+0b5ih4BdS68Cgas4xJ7g=; b=L1+WKY7aSHpMCN1DypXOl9l8Yf4j9uD2qofAYYpgIbBjcCOqnlTv92QjHa+LjjqQ1i 50+uh0uwrC8NwnQ/bcOIQ+mZETolvLaFCyO4HYJslvJmixveY3Gf9ds1zceuiiq9vKBI pcHJhY8OIF0OVmWRPeO0wuyVtGQBF6WeBSP31fbQH/gSW7tw7wN2i1tGyy9t8Z/rNCtr 8AL/ZZdTTelsvqIeHqvwalOiVoXmapSFFU3BIP812k8OgyPT/PDCnM+fd8RJ7Ltwfkuz 9iMLU4hMh9WtCVmEz3gZLAjF6VzLyBPrAagaJj8CPw9rKILiaHvOVVhRhCNNRdgiVul7 8tGQ== X-Gm-Message-State: AOJu0Yz2EMPasJgdBIFBfbdduZZ3du0NbymRgFdLszKUf7G6lOoMJpQz 2mQbnZeO38j3wXt65pBAPzo= X-Google-Smtp-Source: AGHT+IEXYlVGR26h67lZ0CQFtDdopC6qPtwRiCX8u/eS0Le2sSi4IxhnE11TM3OyRpNoFH8rv8Esgw== X-Received: by 2002:a81:7b8a:0:b0:59b:c805:de60 with SMTP id w132-20020a817b8a000000b0059bc805de60mr15386292ywc.45.1696869076997; Mon, 09 Oct 2023 09:31:16 -0700 (PDT) Received: from localhost ([2607:fb90:be22:da0:a050:8c3a:c782:514b]) by smtp.gmail.com with ESMTPSA id g139-20020a0ddd91000000b00589a999038asm3797246ywe.77.2023.10.09.09.31.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Oct 2023 09:31:16 -0700 (PDT) Date: Mon, 9 Oct 2023 09:31:15 -0700 From: Yury Norov To: Alexander Lobakin Cc: Andy Shevchenko , Rasmus Villemoes , Alexander Potapenko , Jakub Kicinski , Eric Dumazet , David Ahern , Przemek Kitszel , Simon Horman , netdev@vger.kernel.org, linux-btrfs@vger.kernel.org, dm-devel@redhat.com, ntfs3@lists.linux.dev, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 09/14] bitmap: extend bitmap_{get,set}_value8() to bitmap_{get,set}_bits() Message-ID: References: <20231009151026.66145-1-aleksander.lobakin@intel.com> <20231009151026.66145-10-aleksander.lobakin@intel.com> Precedence: bulk X-Mailing-List: ntfs3@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231009151026.66145-10-aleksander.lobakin@intel.com> + Alexander Potapenko On Mon, Oct 09, 2023 at 05:10:21PM +0200, Alexander Lobakin wrote: > Sometimes there's need to get a 8/16/...-bit piece of a bitmap at a > particular offset. Currently, there are only bitmap_{get,set}_value8() > to do that for 8 bits and that's it. And also a series from Alexander Potapenko, which I really hope will get into the -next really soon. It introduces bitmap_read/write which can set up to BITS_PER_LONG at once, with no limitations on alignment of position and length: https://lore.kernel.org/linux-arm-kernel/ZRXbOoKHHafCWQCW@yury-ThinkPad/T/#mc311037494229647088b3a84b9f0d9b50bf227cb Can you consider building your series on top of it? > Instead of introducing a separate pair for u16 and so on, which doesn't > scale well, extend the existing functions to be able to pass the wanted > value width. Make both offset and width arbitrary, but in order to not > over complicate the current logic and keep the helpers as optimized as > the current ones, require the width to be a pow-2 value and the offset > to be a multiple of the width, while the target piece should not cross > a %BITS_PER_LONG boundary and stay within one long. > Avoid adjusting all the already existing callsites by defining oneliner > wrapper macros named after the former functions. bloat-o-meter shows > almost no difference (+1-2 bytes in a couple of places), meaning the > new helpers get optimized just nicely. > > Reviewed-by: Przemek Kitszel > Signed-off-by: Alexander Lobakin > --- > include/linux/bitmap.h | 51 ++++++++++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 14 deletions(-) > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index 63e422f8ba3d..9c010a7fa331 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -6,8 +6,10 @@ > > #include > #include > +#include > #include > #include > +#include > #include > #include > > @@ -569,38 +571,59 @@ static inline void bitmap_from_u64(unsigned long *dst, u64 mask) > } > > /** > - * bitmap_get_value8 - get an 8-bit value within a memory region > + * bitmap_get_bits - get a 8/16/32/64-bit value within a memory region > * @map: address to the bitmap memory region > - * @start: bit offset of the 8-bit value; must be a multiple of 8 > + * @start: bit offset of the value; must be a multiple of @len > + * @len: bit width of the value; must be a power of two > * > - * Returns the 8-bit value located at the @start bit offset within the @src > - * memory region. > + * Return: the 8/16/32/64-bit value located at the @start bit offset within > + * the @src memory region. Its position (@start + @len) can't cross > + * a ``BITS_PER_LONG`` boundary. > */ > -static inline unsigned long bitmap_get_value8(const unsigned long *map, > - unsigned long start) > +static inline unsigned long bitmap_get_bits(const unsigned long *map, > + unsigned long start, size_t len) > { > const size_t index = BIT_WORD(start); > const unsigned long offset = start % BITS_PER_LONG; > > - return (map[index] >> offset) & 0xFF; > + if (WARN_ON_ONCE(!is_power_of_2(len) || offset % len || > + offset + len > BITS_PER_LONG)) > + return 0; > + > + return (map[index] >> offset) & GENMASK(len - 1, 0); > } > > /** > - * bitmap_set_value8 - set an 8-bit value within a memory region > + * bitmap_set_bits - set a 8/16/32/64-bit value within a memory region > * @map: address to the bitmap memory region > - * @value: the 8-bit value; values wider than 8 bits may clobber bitmap > - * @start: bit offset of the 8-bit value; must be a multiple of 8 > + * @start: bit offset of the value; must be a multiple of @len > + * @value: new value to set > + * @len: bit width of the value; must be a power of two > + * > + * Replaces the 8/16/32/64-bit value located at the @start bit offset within > + * the @src memory region with the new @value. Its position (@start + @len) > + * can't cross a ``BITS_PER_LONG`` boundary. > */ > -static inline void bitmap_set_value8(unsigned long *map, unsigned long value, > - unsigned long start) > +static inline void bitmap_set_bits(unsigned long *map, unsigned long start, > + unsigned long value, size_t len) > { > const size_t index = BIT_WORD(start); > const unsigned long offset = start % BITS_PER_LONG; > + unsigned long mask = GENMASK(len - 1, 0); > > - map[index] &= ~(0xFFUL << offset); > - map[index] |= value << offset; > + if (WARN_ON_ONCE(!is_power_of_2(len) || offset % len || > + offset + len > BITS_PER_LONG)) > + return; > + > + map[index] &= ~(mask << offset); > + map[index] |= (value & mask) << offset; > } > > +#define bitmap_get_value8(map, start) \ > + bitmap_get_bits(map, start, BITS_PER_BYTE) > +#define bitmap_set_value8(map, value, start) \ > + bitmap_set_bits(map, start, value, BITS_PER_BYTE) > + > #endif /* __ASSEMBLY__ */ > > #endif /* __LINUX_BITMAP_H */ > -- > 2.41.0