On Tue, 2015-09-01 at 22:57 -0500, Zhao Qiang-B45475 wrote:

On Wed, 2015-09-02 at 10:33AM -0500, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 02, 2015 11:09 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061; benh@kernel.crashing.org; Li
Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 1/3] genalloc:support memory-allocation with
bytes-alignment to genalloc

On Tue, 2015-09-01 at 22:05 -0500, Zhao Qiang-B45475 wrote:

quoted

On Wed, 2015-09-02 at 10:33AM -0500, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 02, 2015 10:33 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061; benh@kernel.crashing.org;
Li Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 1/3] genalloc:support memory-allocation with
bytes-alignment to genalloc

On Tue, 2015-09-01 at 21:29 -0500, Zhao Qiang-B45475 wrote:

quoted

On Wed, 2015-09-02 at 10:18AM -0500, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 02, 2015 10:18 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061;
benh@kernel.crashing.org; Li Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 1/3] genalloc:support memory-allocation
with bytes-alignment to genalloc

On Tue, 2015-09-01 at 21:10 -0500, Zhao Qiang-B45475 wrote:

quoted

On Wed, 2015-09-02 at 08:38AM +0800, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 02, 2015 8:30 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org; lauraa@codeaurora.org; Xie
Xiaobo-R63061; benh@kernel.crashing.org; Li Yang-Leo-R58472;
paulus@samba.org
Subject: Re: [PATCH V7 1/3] genalloc:support
memory-allocation with bytes-alignment to genalloc

On Mon, 2015-08-31 at 16:58 +0800, Zhao Qiang wrote:

quoted

Bytes alignment is required to manage some special RAM, so
add gen_pool_first_fit_align to genalloc, meanwhile add
gen_pool_alloc_data to pass data to
gen_pool_first_fit_align(modify gen_pool_alloc as a
wrapper)

Signed-off-by: Zhao Qiang <redacted>
---
Changes for v6:
      - patches set v6 include a new patch because of using
      - genalloc to manage QE MURAM, patch 0001 is the new
      - patch, adding bytes alignment for allocation for

use.

quoted

quoted

quoted

quoted

quoted

quoted

quoted

Changes for v7:
      - cpm muram also need to use genalloc to manage, it

has

quoted

quoted

quoted

quoted

quoted

quoted

quoted

        a function to reserve a specific region of muram,
        add offset to genpool_data for start addr to be

allocated.

quoted

quoted

quoted

quoted

This seems to be describing more than just the changes in
this

patch.

quoted

quoted

quoted

quoted

What does also handling cpm have to do with this patch?  Are
you adding support for reserving a specific region in this
patch?  I don't see it, and in any case it should go in a

different patch.

quoted

quoted

quoted

quoted

quoted

Yes, I added. The code below can support the function.
      offset_bit = (alignment->offset + (1UL << order) - 1) >>

order;

quoted

quoted

quoted

      return bitmap_find_next_zero_area(map, size, start +
offset_bit,

nr,

quoted

                        align_mask);

CPM has an function cpm_muram_alloc_fixed, needing to allocate
muram from a Specific offset. So I add the code and add offset
to

struct data.

quoted

quoted

I thought the offset was related to the previous discussion of
checking for allocation failure.  Are you using it to implement
alloc_fixed()?  If so, please don't.  Besides the awkward
implementation (what does it logically have to do with
gen_pool_first_fit_align?), it does not appear to be correct -
- what happens with multiple chunks?  What happens if part of
the region the caller is trying to reserve is already taken?
Implement a proper function to reserve a fixed genalloc region.

This offset is totally different with the workaround OFFSET!

There's a reason why we write changelogs that describe what the
patch is doing, and avoid combining logically distinct changes in the

same patch.

quoted

quoted

quoted

This offset is the offset of the muram.

The offset of the muram relative to what?  Or do you mean the offset
into muram?

Yes, the offset into muram.

quoted

quoted

CPM need to allocate block from a specific offset due to hardware
restriction.
So I must handle this offset in genalloc.

Again, if you need to be able to mark a specific range reserved, add
a function that does that properly.  Don't try to hack it in the way
you did.

Add a function? Do you mean add a new alloc function or new algo?
If you mean new algo, CPM use both align algo and new algo, set
Different algos in different muram_alloc func?

I was thinking that it was a sufficiently different operation that it
warranted its own independent function, but I suppose you could do it as
an algorithm that only accepts the requested range and returns failure
for all other chunks (as well as if the range is unavailable).  It would
not be related at all to the aligned-alloc algorithm.

If do so, I need set algo in different muram_alloc function, it is 
redundancy.

If you do it as a separate top-level function there would be no algorithm.

Using an algorithm would be simpler to implement, but a bit more awkward in 
the caller due to the need to swap out the algorithm (unless we change 
gen_pool_alloc_data to gen_pool_alloc_algo_data or similar...).

The algos has start para, but it set start_bit = 0 in gen_pool_alloc_data, 
can We pass a start addr para to gen_pool_alloc_data?

No.  Again, setting that "start" variable is not equivalent to what you're 
trying to accomplish, even if it happens to work in your test case.

-Scott