On Fri, Aug 14, 2020 at 9:15 AM Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote: > Thanks for the patch. > > On Fri, 14 Aug 2020 at 03:25, John Stultz <john.stultz@xxxxxxxxxx> wrote: > > > > This adds a heap that allocates non-contiguous buffers that are > > marked as writecombined, so they are not cached by the CPU. > > > > What's the rationale for exposing the memory > attribute as a new heap, instead of just introducing flags? > > I guess this calls for some guidelines on what situations > call for a separate heap, and when it's just a modifier flag. YES! :) A big part of this patch is to start a discussion and feel out what properties of heaps are generic enough to be flags, and what aspects are unique enough to deserve having their own heap implementation. ION used the ION_FLAG_CACHED bit for this and considered it a generic property (though by default all buffers were uncached). This seemed to cause enough friction in reviews that we dropped it and used cachable buffers for the initial DMA BUF heaps. Further, I want to make sure we avoid the custom flag abuse that ION saw, especially with vendor heaps. So I think having each unique behavior being a separate heap is a reasonable stance. That said, we added the (currently unused) heap-flags field to the interface as there may be some attributes or modalities that are truly generic across heaps. So if we want to add an UNCACHED flag instead, I'm open to that.. however I want to make sure it has clear general meaning so that its behavior is consistent across all heaps and architectures (or produces an error if it's not supported). thanks -john
