linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jaewon Kim <jaewon31.kim@samsung.com>
To: Yury Norov <ynorov@caviumnetworks.com>
Cc: gregkh@linuxfoundation.org, akpm@linux-foundation.org,
	labbott@redhat.com, mina86@mina86.com, m.szyprowski@samsung.com,
	gregory.0xf0@gmail.com, laurent.pinchart@ideasonboard.com,
	akinobu.mita@gmail.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, jaewon31.kim@gmail.com
Subject: Re: [PATCH] lib: bitmap: introduce bitmap_find_next_zero_area_and_size
Date: Tue, 17 Jan 2017 12:22:13 +0900	[thread overview]
Message-ID: <587D8DE5.6040408@samsung.com> (raw)
In-Reply-To: <20170115071726.GB6474@yury-N73SV>



On 2017년 01월 15일 16:17, Yury Norov wrote:
> Hi Jaewon,
>
> with all comments above, some of my concerns.
>
> On Mon, Dec 26, 2016 at 01:18:11PM +0900, Jaewon Kim wrote:
>> There was no bitmap API which returns both next zero index and size of zeros
>> from that index.
> Yes, there is. Most probably because this function is not needed.
> Typical usecase is looking for the area of N free bits, were caller
> knows N, and doesn't care of free areas smaller than N. There is 
> bitmap_find_next_zero_area() for exactly that.
Hi Yuri
Thank you for comment.
I did not mean finding free area but wanted to know its size.
So bitmap_find_next_zero_area is not what I wanted.
I will not submit this patch though.
>
>> This is helpful to look fragmentation. This is an test code to look size of zeros.
>> Test result is '10+9+994=>1013 found of total: 1024'
>>
>> unsigned long search_idx, found_idx, nr_found_tot;
>> unsigned long bitmap_max;
>> unsigned int nr_found;
>> unsigned long *bitmap;
>>
>> search_idx = nr_found_tot = 0;
>> bitmap_max = 1024;
>> bitmap = kzalloc(BITS_TO_LONGS(bitmap_max) * sizeof(long),
>> 		 GFP_KERNEL);
>>
>> /* test bitmap_set offset, count */
>> bitmap_set(bitmap, 10, 1);
>> bitmap_set(bitmap, 20, 10);
>>
>> for (;;) {
>> 	found_idx = bitmap_find_next_zero_area_and_size(bitmap,
>> 				bitmap_max, search_idx, &nr_found);
>> 	if (found_idx >= bitmap_max)
>> 		break;
>> 	if (nr_found_tot == 0)
>> 		printk("%u", nr_found);
>> 	else
>> 		printk("+%u", nr_found);
>> 	nr_found_tot += nr_found;
>> 	search_idx = found_idx + nr_found;
>> }
>> printk("=>%lu found of total: %lu\n", nr_found_tot, bitmap_max);
>>
>> Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com>
> This usecase is problematic in real world. Consider 1-byte bitmap
> '01010101'. To store fragmentation information for further analysis,
> you have to allocate 4 pairs of address and size. On 64-bit machine
> it's 64 bytes of additional memory. Brief grepping of kernel sources
> shows that no one does it. Correct me if I missed something.
Sorry but I did not understand for "you have to allocate 4 pairs of address and size"
I used just local variables.
>
> If you still think this API is useful, you'd walk over kernel
> and find bins of code that will become better with your function,
> and send the patch that adds the use of your function there. Probable
> candidates for search are bitmap_find_next_zero_area() and find_next_bit()
> functions.
>
> If the only suitable place for new function is your example below, I
> think it's better not to introduce new API and reconsider your
> implementation instead.
>
> Yury.
>
>
>

  reply	other threads:[~2017-01-17  3:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20161226041809epcas5p1981244de55764c10f1a80d80346f3664@epcas5p1.samsung.com>
2016-12-26  4:18 ` [PATCH] lib: bitmap: introduce bitmap_find_next_zero_area_and_size Jaewon Kim
2016-12-26 21:09   ` Michal Nazarewicz
2016-12-27  4:14     ` Jaewon Kim
2016-12-27 10:05   ` Michal Hocko
2016-12-28  4:41     ` Jaewon Kim
2016-12-28  8:32       ` Michal Hocko
2016-12-28 14:14       ` Michal Nazarewicz
2016-12-29  2:13         ` Jaewon Kim
2017-01-15  7:17   ` Yury Norov
2017-01-17  3:22     ` Jaewon Kim [this message]
2017-01-16 17:59   ` Andy Shevchenko

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=587D8DE5.6040408@samsung.com \
    --to=jaewon31.kim@samsung.com \
    --cc=akinobu.mita@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gregory.0xf0@gmail.com \
    --cc=jaewon31.kim@gmail.com \
    --cc=labbott@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mina86@mina86.com \
    --cc=ynorov@caviumnetworks.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).