On 07/03/18 16:23, Bart Van Assche wrote:
On Wed, 2018-03-07 at 12:47 +0000, Tvrtko Ursulin wrote:
We can derive the order from sg->length and so do not need to pass it in
explicitly. Rename the function to sgl_free_n.
Using get_order() to compute the order looks fine to me but this patch will
have to rebased in order to address the comments on the previous patches.
Ok I guess my main questions are the ones from the cover letter - where
is this API going and why did it get in a bit of a funky state? Because
it doesn't look fully thought through and tested to me.
My motivation is that I would like to extend it to add
sgl_alloc_order_min_max, which takes min order and max order, and
allocates as large chunks as it can given those constraints. This is
something we have in i915 and could then drop our implementation and use
the library function.
But I also wanted to refactor sgl_alloc_order to benefit from the
existing chained struct scatterlist allocator. But SGL API does not
embed into struct sg_table, neither it carries explicitly the number of
nents allocated, making it impossible to correctly free with existing
sg_free_table.
Another benefit of using the existing sg allocator would be that for
large allocation you don't depend on the availability of contiguous
chunks like you do with kmalloc_array.
For instance if in another reply you mentioned 4GiB allocations are a
possibility. If you use order 0 that means you need 1M nents, which can
be something like 32 bytes each and you need a 32MiB kmalloc for the
nents array and thats quite big. If you would be able to reuse the
existing sg_alloc_table infrastructure (I have patches which extract it
if you don't want to deal with struct sg_table), you would benefit from
PAGE_SIZE allocations.
Also I am not sure if a single gfp argument to sgl_alloc_order is the
right thing to do. I have a feeling you either need to ignore it for
kmalloc_array, or pass in two gfp_t arguments to be used for metadata
and backing storage respectively.
So I have many questions regarding the current state and future
direction, but essentially would like to make it usable for other
drivers, like i915, as well.
Regards,
Tvrtko
