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@mediatek.com 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@amd.com wrote:
Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> -----Original Message----- > From: dri-devel dri-devel-bounces@lists.freedesktop.org > On > Behalf Of > Ruhl, Michael J > > -----Original Message----- > > From: dri-devel < > > dri-devel-bounces@lists.freedesktop.org> > > On Behalf Of > > guangming.cao@mediatek.com > > + /* > > + * 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@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek