All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dennis Zhou <dennis@kernel.org>
To: Yury Norov <yury.norov@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-mmc@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Tejun Heo <tj@kernel.org>, Steven Rostedt <rostedt@goodmis.org>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Christoph Lameter <cl@linux.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 2/4] bitmap: unify find_bit operations
Date: Wed, 21 Jul 2021 21:12:05 +0000	[thread overview]
Message-ID: <YPiNpQskvmuGd/qW@google.com> (raw)
In-Reply-To: <20210719021755.883182-3-yury.norov@gmail.com>

On Sun, Jul 18, 2021 at 07:17:53PM -0700, Yury Norov wrote:
> bitmap_for_each_{set,clear}_region() are similar to for_each_bit()
> macros in include/linux/find.h, but interface and implementation
> of them are different.
> 
> This patch adds for_each_bitrange() macros and drops unused
> bitmap_*_region() API in sake of unification.
> 
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  drivers/mmc/host/renesas_sdhi_core.c |  2 +-
>  include/linux/bitmap.h               | 41 --------------------
>  include/linux/find.h                 | 56 ++++++++++++++++++++++++++++
>  mm/percpu.c                          | 20 ++++------
>  4 files changed, 65 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index e49ca0f7fe9a..efd33b1fc467 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -647,7 +647,7 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host)
>  	 * is at least SH_MOBILE_SDHI_MIN_TAP_ROW probes long then use the
>  	 * center index as the tap, otherwise bail out.
>  	 */
> -	bitmap_for_each_set_region(bitmap, rs, re, 0, taps_size) {
> +	for_each_set_bitrange(rs, re, bitmap, taps_size) {
>  		if (re - rs > tap_cnt) {
>  			tap_end = re;
>  			tap_start = rs;
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 3f7c6731b203..96670abf49bd 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -55,12 +55,6 @@ struct device;
>   *  bitmap_clear(dst, pos, nbits)               Clear specified bit area
>   *  bitmap_find_next_zero_area(buf, len, pos, n, mask)  Find bit free area
>   *  bitmap_find_next_zero_area_off(buf, len, pos, n, mask, mask_off)  as above
> - *  bitmap_next_clear_region(map, &start, &end, nbits)  Find next clear region
> - *  bitmap_next_set_region(map, &start, &end, nbits)  Find next set region
> - *  bitmap_for_each_clear_region(map, rs, re, start, end)
> - *  						Iterate over all clear regions
> - *  bitmap_for_each_set_region(map, rs, re, start, end)
> - *  						Iterate over all set regions
>   *  bitmap_shift_right(dst, src, n, nbits)      *dst = *src >> n
>   *  bitmap_shift_left(dst, src, n, nbits)       *dst = *src << n
>   *  bitmap_cut(dst, src, first, n, nbits)       Cut n bits from first, copy rest
> @@ -459,41 +453,6 @@ static inline void bitmap_replace(unsigned long *dst,
>  		__bitmap_replace(dst, old, new, mask, nbits);
>  }
>  
> -static inline void bitmap_next_clear_region(unsigned long *bitmap,
> -					    unsigned int *rs, unsigned int *re,
> -					    unsigned int end)
> -{
> -	*rs = find_next_zero_bit(bitmap, end, *rs);
> -	*re = find_next_bit(bitmap, end, *rs + 1);
> -}
> -
> -static inline void bitmap_next_set_region(unsigned long *bitmap,
> -					  unsigned int *rs, unsigned int *re,
> -					  unsigned int end)
> -{
> -	*rs = find_next_bit(bitmap, end, *rs);
> -	*re = find_next_zero_bit(bitmap, end, *rs + 1);
> -}
> -
> -/*
> - * Bitmap region iterators.  Iterates over the bitmap between [@start, @end).
> - * @rs and @re should be integer variables and will be set to start and end
> - * index of the current clear or set region.
> - */
> -#define bitmap_for_each_clear_region(bitmap, rs, re, start, end)	     \
> -	for ((rs) = (start),						     \
> -	     bitmap_next_clear_region((bitmap), &(rs), &(re), (end));	     \
> -	     (rs) < (re);						     \
> -	     (rs) = (re) + 1,						     \
> -	     bitmap_next_clear_region((bitmap), &(rs), &(re), (end)))
> -
> -#define bitmap_for_each_set_region(bitmap, rs, re, start, end)		     \
> -	for ((rs) = (start),						     \
> -	     bitmap_next_set_region((bitmap), &(rs), &(re), (end));	     \
> -	     (rs) < (re);						     \
> -	     (rs) = (re) + 1,						     \
> -	     bitmap_next_set_region((bitmap), &(rs), &(re), (end)))
> -
>  /**
>   * BITMAP_FROM_U64() - Represent u64 value in the format suitable for bitmap.
>   * @n: u64 value
> diff --git a/include/linux/find.h b/include/linux/find.h
> index ae9ed52b52b8..5bb6db213bcb 100644
> --- a/include/linux/find.h
> +++ b/include/linux/find.h
> @@ -301,6 +301,62 @@ unsigned long find_next_bit_le(const void *addr, unsigned
>  	     (bit) < (size);					\
>  	     (bit) = find_next_zero_bit((addr), (size), (bit) + 1))
>  
> +/**
> + * for_each_set_bitrange - iterate over all set bit ranges [b; e)
> + * @b: bit offset of start of current bitrange (first set bit)
> + * @e: bit offset of end of current bitrange (first unset bit)
> + * @addr: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + */
> +#define for_each_set_bitrange(b, e, addr, size)			\
> +	for ((b) = find_next_bit((addr), (size), 0),		\
> +	     (e) = find_next_zero_bit((addr), (size), (b) + 1);	\
> +	     (b) < (size);					\
> +	     (b) = find_next_bit((addr), (size), (e) + 1),	\
> +	     (e) = find_next_zero_bit((addr), (size), (b) + 1))
> +
> +/**
> + * for_each_set_bitrange_from - iterate over set bit ranges [b; e)
> + * @b: bit offset of start of current bitrange (first set bit); must be initialized
> + * @e: bit offset of end of current bitrange (first unset bit)
> + * @addr: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + */
> +#define for_each_set_bitrange_from(b, e, addr, size)		\
> +	for ((b) = find_next_bit((addr), (size), (b)),		\
> +	     (e) = find_next_zero_bit((addr), (size), (b) + 1);	\
> +	     (b) < (size);					\
> +	     (b) = find_next_bit((addr), (size), (e) + 1),	\
> +	     (e) = find_next_zero_bit((addr), (size), (b) + 1))
> +
> +/**
> + * for_each_clear_bitrange - iterate over all unset bit ranges [b; e)
> + * @b: bit offset of start of current bitrange (first unset bit)
> + * @e: bit offset of end of current bitrange (first set bit)
> + * @addr: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + */
> +#define for_each_clear_bitrange(b, e, addr, size)		\
> +	for ((b) = find_next_zero_bit((addr), (size), 0),	\
> +	     (e) = find_next_bit((addr), (size), (b) + 1);	\
> +	     (b) < (size);					\
> +	     (b) = find_next_zero_bit((addr), (size), (e) + 1),	\
> +	     (e) = find_next_bit((addr), (size), (b) + 1))
> +
> +/**
> + * for_each_clear_bitrange_from - iterate over unset bit ranges [b; e)
> + * @b: bit offset of start of current bitrange (first unset bit); must be initialized
> + * @e: bit offset of end of current bitrange (first set bit)
> + * @addr: bitmap address to base the search on
> + * @size: bitmap size in number of bits
> + */
> +#define for_each_clear_bitrange_from(b, e, addr, size)		\
> +	for ((b) = find_next_zero_bit((addr), (size), (b)),	\
> +	     (e) = find_next_bit((addr), (size), (b) + 1);	\
> +	     (b) < (size);					\
> +	     (b) = find_next_zero_bit((addr), (size), (e) + 1),	\
> +	     (e) = find_next_bit((addr), (size), (b) + 1))
> +
>  /**
>   * for_each_set_clump8 - iterate over bitmap for each 8-bit clump with set bits
>   * @start: bit offset to start search and to store the current iteration offset
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 25461571dcc5..6d518e822983 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -780,7 +780,7 @@ static void pcpu_block_refresh_hint(struct pcpu_chunk *chunk, int index)
>  {
>  	struct pcpu_block_md *block = chunk->md_blocks + index;
>  	unsigned long *alloc_map = pcpu_index_alloc_map(chunk, index);
> -	unsigned int rs, re, start;	/* region start, region end */
> +	unsigned int start, end;	/* region start, region end */
>  
>  	/* promote scan_hint to contig_hint */
>  	if (block->scan_hint) {
> @@ -796,9 +796,8 @@ static void pcpu_block_refresh_hint(struct pcpu_chunk *chunk, int index)
>  	block->right_free = 0;
>  
>  	/* iterate over free areas and update the contig hints */
> -	bitmap_for_each_clear_region(alloc_map, rs, re, start,
> -				     PCPU_BITMAP_BLOCK_BITS)
> -		pcpu_block_update(block, rs, re);
> +	for_each_clear_bitrange_from(start, end, alloc_map, PCPU_BITMAP_BLOCK_BITS)
> +		pcpu_block_update(block, start, end);
>  }
>  
>  /**
> @@ -1856,13 +1855,12 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
>  
>  	/* populate if not all pages are already there */
>  	if (!is_atomic) {
> -		unsigned int page_start, page_end, rs, re;
> +		unsigned int page_end, rs, re;
>  
> -		page_start = PFN_DOWN(off);
> +		rs = PFN_DOWN(off);
>  		page_end = PFN_UP(off + size);
>  
> -		bitmap_for_each_clear_region(chunk->populated, rs, re,
> -					     page_start, page_end) {
> +		for_each_clear_bitrange_from(rs, re, chunk->populated, page_end) {
>  			WARN_ON(chunk->immutable);
>  
>  			ret = pcpu_populate_chunk(chunk, rs, re, pcpu_gfp);
> @@ -2018,8 +2016,7 @@ static void pcpu_balance_free(bool empty_only)
>  	list_for_each_entry_safe(chunk, next, &to_free, list) {
>  		unsigned int rs, re;
>  
> -		bitmap_for_each_set_region(chunk->populated, rs, re, 0,
> -					   chunk->nr_pages) {
> +		for_each_set_bitrange(rs, re, chunk->populated, chunk->nr_pages) {
>  			pcpu_depopulate_chunk(chunk, rs, re);
>  			spin_lock_irq(&pcpu_lock);
>  			pcpu_chunk_depopulated(chunk, rs, re);
> @@ -2089,8 +2086,7 @@ static void pcpu_balance_populated(void)
>  			continue;
>  
>  		/* @chunk can't go away while pcpu_alloc_mutex is held */
> -		bitmap_for_each_clear_region(chunk->populated, rs, re, 0,
> -					     chunk->nr_pages) {
> +		for_each_clear_bitrange(rs, re, chunk->populated, chunk->nr_pages) {
>  			int nr = min_t(int, re - rs, nr_to_pop);
>  
>  			spin_unlock_irq(&pcpu_lock);
> -- 
> 2.30.2
> 

Acked-by: Dennis Zhou <dennis@kernel.org>

Thanks,
Dennis

  reply	other threads:[~2021-07-21 21:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19  2:17 [PATCH v2 0/4] bitmap: unify for_each_bit() macros Yury Norov
2021-07-19  2:17 ` [PATCH 1/4] mm/percpu: micro-optimize pcpu_is_populated() Yury Norov
2021-07-21 21:07   ` Dennis Zhou
2021-07-19  2:17 ` [PATCH 2/4] bitmap: unify find_bit operations Yury Norov
2021-07-21 21:12   ` Dennis Zhou [this message]
2021-08-04 10:07   ` Ulf Hansson
2021-08-04 10:07     ` Ulf Hansson
2021-08-11  7:38   ` Wolfram Sang
2021-08-12  1:23     ` Yury Norov
2021-08-12  1:23       ` Yury Norov
2021-07-19  2:17 ` [PATCH 3/4] lib: bitmap: add performance test for bitmap_print_to_pagebuf Yury Norov
2021-07-19  2:17 ` [PATCH 4/4] vsprintf: rework bitmap_list_string Yury Norov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YPiNpQskvmuGd/qW@google.com \
    --to=dennis@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tj@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=wsa+renesas@sang-engineering.com \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.