Hi,
I was writing the test module, for layered bitmaps, specific to my
requirement such that I want to have control/track of 2GB virtual
address space. As per the discussion with Paul Mundt, I have taken the
approach of segmenting the virt. space with 128MB of blocks and then
to do find_next_zero_bit followed by bitmap_find_free_region on the
bitmaps.
I have attached the test module code. I have confusion about the what
to pass as second argument of bitmap_find_free_region for 2nd layer
bitmap.
Do we have to pass "total size of the block...here 128MB" as the
second argument or no. of bits in that block, which 32768 (128 >>
PAGE_SHIFT) ?
This confusion arised as store queue implementation (sq.c) of sh arch,
passes total size of the bitmap (64M) as the second argument instead
of bits.
Same is the case with bitmap_full, where I have to pass total size of
the block (here 128MB) instead of no. of bits, in-order it to make my
test module work correctly.
--
Link: sq.c
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;h=7bcc73f9b8df535ad8887383aeac71a8b2491ef5;hb=0215ffb08ce99e2bb59eca114a99499a4d06e704;f=arch/sh/kernel/cpu/sh4/sq.c
--
---Komal Shah
http://komalshah.blogspot.com
M. R. Brown and Paul Mundt -- take a look at the question at end of this post.
Komal wrote:
>
> I have attached the test module code. I have confusion about the what
> to pass as second argument of bitmap_find_free_region for 2nd layer
> bitmap.
>
> Do we have to pass "total size of the block...here 128MB" as the
> second argument or no. of bits in that block, which 32768 (128 >>
> PAGE_SHIFT) ?
>
> This confusion arised as store queue implementation (sq.c) of sh arch,
> passes total size of the bitmap (64M) as the second argument instead
> of bits.
>
> Same is the case with bitmap_full, where I have to pass total size of
> the block (here 128MB) instead of no. of bits, in-order it to make my
> test module work correctly.
A bitmap is essentially an array of bits. The bitmap_* routines should
take a count of how many bits are in the bitmap array (or in your case,
how many bits are in the segment that you expect to be used in that call.)
So I'm guessing you will be passing L2_BM_BITS for that second argument.
The call to bitmap_find_free_region(), in arch/sh/kernel/cpu/sh4/sq.c
looks bogus:
page = bitmap_find_free_region(sq_bitmap, 0x04000000,
get_order(map->size));
It says the bitmap has 0x04000000 bits. This would take 0x04000000 / 8
which is 8388608 (decimal) bytes to hold. That's an insanely
huge bitmap - 8 million bytes worth of bits.
I can't make entire sense of the the code you attached. I get confused
over the various sizes of maps and what the maps represent, and of bits
and bytes, and of the two levels.
When you do call bitmap_find_free_region(), you should pass the number
of bits that are in that segment of the L2 bitmap, which I would guess
is L2_BM_BITS (32 K).
When you kzalloc() the L2 bitmap for all 16 segments at once, you
should pass the number of -bytes-, not the number of longs. That is,
I guess you want to replace your line:
bm_l2 = kzalloc(size * L1_BM_NUM_SEGS, GFP_KERNEL);
with the line:
bm_l2 = kzalloc(size * L1_BM_NUM_SEGS * sizeof(long), GFP_KERNEL);
This is because, like most of the *alloc() routines, kzalloc() takes
a byte count, not a long count. The related DECLARE_BITMAP() macro
computes the number of longs, not bytes, but that is because it is
declaring an array of longs, not allocating a number of bytes.
But I might be entirely confused about what this code is doing.
If this code is to become a kernel patch proposal, it will take some
work to make it easier to understand.
See also the code and comments in lib/bitmap.c for the routine
bitmap_find_free_region() to see what it is doing.
And, as I noted above, I suspect that the arch sh code using
these bitmap routines is seriously wrong, and will blow up if
ever asked to handle the harder, more full, cases.
M. R. Brown and Paul Mundt - looks like you two are listed as authors
for that sh arch code for "SH-4 integrated Store Queues".
Could you take a look at it, and see whether it is that code, or my
brain, that is on drugs?
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On Thu, Nov 30, 2006 at 04:10:08PM -0800, Paul Jackson wrote:
> The call to bitmap_find_free_region(), in arch/sh/kernel/cpu/sh4/sq.c
> looks bogus:
>
> page = bitmap_find_free_region(sq_bitmap, 0x04000000,
> get_order(map->size));
>
> It says the bitmap has 0x04000000 bits. This would take 0x04000000 / 8
> which is 8388608 (decimal) bytes to hold. That's an insanely
> huge bitmap - 8 million bytes worth of bits.
>
Ouch, you're right, for some reason we missed the >> PAGE_SHIFT here,
even though it was handled properly in sq_api_init(). I suppose we
haven't hit this in practice as most of the users end up being quite
small. I'll fix it up. Good catch, thanks!
Paul Mundt wrote:
> I'll fix it up. Good catch, thanks!
You're welcome.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On 12/1/06, Paul Jackson <[email protected]> wrote:
>
> M. R. Brown and Paul Mundt -- take a look at the question at end of this post.
>
> Komal wrote:
> >
> > I have attached the test module code. I have confusion about the what
> > to pass as second argument of bitmap_find_free_region for 2nd layer
> > bitmap.
> >
> > Do we have to pass "total size of the block...here 128MB" as the
> > second argument or no. of bits in that block, which 32768 (128 >>
> > PAGE_SHIFT) ?
> >
> > This confusion arised as store queue implementation (sq.c) of sh arch,
> > passes total size of the bitmap (64M) as the second argument instead
> > of bits.
> >
> > Same is the case with bitmap_full, where I have to pass total size of
> > the block (here 128MB) instead of no. of bits, in-order it to make my
> > test module work correctly.
>
> A bitmap is essentially an array of bits. The bitmap_* routines should
> take a count of how many bits are in the bitmap array (or in your case,
> how many bits are in the segment that you expect to be used in that call.)
>
> So I'm guessing you will be passing L2_BM_BITS for that second argument.
>
> The call to bitmap_find_free_region(), in arch/sh/kernel/cpu/sh4/sq.c
> looks bogus:
>
> page = bitmap_find_free_region(sq_bitmap, 0x04000000,
> get_order(map->size));
>
> It says the bitmap has 0x04000000 bits. This would take 0x04000000 / 8
> which is 8388608 (decimal) bytes to hold. That's an insanely
> huge bitmap - 8 million bytes worth of bits.
>
> I can't make entire sense of the the code you attached. I get confused
> over the various sizes of maps and what the maps represent, and of bits
> and bytes, and of the two levels.
>
Thanx for clarifying. I am now able to understand it properly, and I
will modify my code, so that it will be easy to understand/read :)
--
---Komal Shah
http://komalshah.blogspot.com
On 12/1/06, Paul Jackson <[email protected]> wrote:
>
> M. R. Brown and Paul Mundt -- take a look at the question at end of this post.
>
> Komal wrote:
> >
> > I have attached the test module code. I have confusion about the what
> > to pass as second argument of bitmap_find_free_region for 2nd layer
> > bitmap.
> >
> > Do we have to pass "total size of the block...here 128MB" as the
> > second argument or no. of bits in that block, which 32768 (128 >>
> > PAGE_SHIFT) ?
> >
> > This confusion arised as store queue implementation (sq.c) of sh arch,
> > passes total size of the bitmap (64M) as the second argument instead
> > of bits.
> >
> > Same is the case with bitmap_full, where I have to pass total size of
> > the block (here 128MB) instead of no. of bits, in-order it to make my
> > test module work correctly.
>
> A bitmap is essentially an array of bits. The bitmap_* routines should
> take a count of how many bits are in the bitmap array (or in your case,
> how many bits are in the segment that you expect to be used in that call.)
>
> So I'm guessing you will be passing L2_BM_BITS for that second argument.
>
> The call to bitmap_find_free_region(), in arch/sh/kernel/cpu/sh4/sq.c
> looks bogus:
>
> page = bitmap_find_free_region(sq_bitmap, 0x04000000,
> get_order(map->size));
>
> It says the bitmap has 0x04000000 bits. This would take 0x04000000 / 8
> which is 8388608 (decimal) bytes to hold. That's an insanely
> huge bitmap - 8 million bytes worth of bits.
>
> I can't make entire sense of the the code you attached. I get confused
> over the various sizes of maps and what the maps represent, and of bits
> and bytes, and of the two levels.
>
> When you do call bitmap_find_free_region(), you should pass the number
> of bits that are in that segment of the L2 bitmap, which I would guess
> is L2_BM_BITS (32 K).
>
> When you kzalloc() the L2 bitmap for all 16 segments at once, you
> should pass the number of -bytes-, not the number of longs. That is,
> I guess you want to replace your line:
>
> bm_l2 = kzalloc(size * L1_BM_NUM_SEGS, GFP_KERNEL);
>
> with the line:
>
> bm_l2 = kzalloc(size * L1_BM_NUM_SEGS * sizeof(long), GFP_KERNEL);
>
> This is because, like most of the *alloc() routines, kzalloc() takes
> a byte count, not a long count. The related DECLARE_BITMAP() macro
> computes the number of longs, not bytes, but that is because it is
> declaring an array of longs, not allocating a number of bytes.
>
> But I might be entirely confused about what this code is doing.
Oops, forgot to reply on this. This code is just a test module
in-order to go for layered bitmaps and in-turn understand bitmap_*
apis. As I said, requirement was to manage 2GB of virtual space of the
co-processor I have attached with ARM11. Now, as we can't allocate
single bitmap for 2GB, Paul Mundt suggested to have a layered bitmaps,
where 1st bitmap segmenting the virtual space into 128MB blocks and
have 2nd layer 128MB size bitmap for each segment in layer 1 bitmap.
Hope this clarifies the motivation behind it.
--
---Komal Shah
http://komalshah.blogspot.com