On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote: > On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao > <guangming.cao@xxxxxxxxxxxx> wrote: > > > > On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote: > > > Am 14.01.22 um 00:26 schrieb John Stultz: > > > > On Thu, Jan 13, 2022 at 5:05 AM Christian König > > > > <christian.koenig@xxxxxxx> wrote: > > > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J: > > > > > > > -----Original Message----- > > > > > > > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> > > > > > > > On > > > > > > > Behalf Of > > > > > > > Ruhl, Michael J > > > > > > > > -----Original Message----- > > > > > > > > From: dri-devel < > > > > > > > > dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> > > > > > > > > On Behalf Of > > > > > > > > guangming.cao@xxxxxxxxxxxx > > > > > > > > + /* > > > > > > > > + * Invalid size check. The "len" should be less > > > > > > > > than > > > > > > > > totalram. > > > > > > > > + * > > > > > > > > + * Without this check, once the invalid size > > > > > > > > allocation > > > > > > > > runs on a process > > > > > > > > that > > > > > > > > + * can't be killed by OOM flow(such as "gralloc" on > > > > > > > > Android devices), it > > > > > > > > will > > > > > > > > + * cause a kernel exception, and to make matters > > > > > > > > worse, > > > > > > > > we can't find > > > > > > > > who are using > > > > > > > > + * so many memory with "dma_buf_debug_show" since > > > > > > > > the > > > > > > > > relevant > > > > > > > > dma-buf hasn't exported. > > > > > > > > + */ > > > > > > > > + if (len >> PAGE_SHIFT > totalram_pages()) > > > > > > > > > > > > > > If your "heap" is from cma, is this still a valid check? > > > > > > > > > > > > And thinking a bit further, if I create a heap from > > > > > > something > > > > > > else (say device memory), > > > > > > you will need to be able to figure out the maximum > > > > > > allowable > > > > > > check for the specific > > > > > > heap. > > > > > > > > > > > > Maybe the heap needs a callback for max size? > > > > Yes, I agree with this solution. > > If dma-heap framework support this via adding a callback to support > > it, > > seems it's more clear than adding a limitation in dma-heap > > framework > > since each heap maybe has different limitation. > > If you prefer adding callback, I can update this patch and add > > totalram > > limitation to system dma-heap. > > If the max value is per-heap, why not enforce that value in the > per-heap allocation function? > > Moving the check to the heap alloc to me seems simpler to me than > adding complexity to the infrastructure to add a heap max_size > callback. Is there some other use for the callback that you envision? > > thanks > -john Thanks for your comment. If you think max the value is per-heap, why not add an optional callback for dma-heap to solve this issue(prevent consuming too much time for a doomed to fail allocation), if the dma-heap doesn't have a special size check, just use the default value(totalram) in dma-heap framework to do the size check. Yes, for linux dma-heaps, only system-heap needs it, so adding it in system heap is the simplest. However, there are many vendor dma-heaps like system-heap which won't be uploaded to linux codebase, and maybe have same limitation, all these heaps need to add the same limitation. I just think it's boring. However, If you think discussing these absent cases based on current linux code is meaningless, I also agree to it. So, to summarize, if you still think adding it in system_heap.c is better, I also agree and I will update the patch to add it in system_heap.c Thanks~ Guangming > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-mediatek
