On Thu, 19 Jul 2018 08:08:06 +0200, Zhang, Jun wrote:
Hello, Takashi
I think use our patch, it's NOT possible that the returned size is over sgbuf->tblsize.
In function snd_malloc_sgbuf_pages,
Pages is align page, sgbuf->tblsize is align 32*page, chunk is align 2^n*page,
in our panic case, pages = 123, tlbsize = 128, 1st loop trunk = 32 2nd loop trunk = 32 3rd loop trunk = 32 4th loop trunk = 16 5th loop trunk = 16 So in 5th loop pages-trunk = -5, which make dead loop.
Looking at the code again, yeah, you are right, that won't happen.
And now it becomes clear: the fundamental problem is that snd_dma_alloc_pages_fallback() returns a larger size than requested. It would be acceptable if the internal allocator aligns a larger size, but it shouldn't appear in the returned size outside. I believe this was just a misunderstanding of get_order() usage there. (BTW, it's interesting that the allocation with a larger block worked while allocation with a smaller chunk failed; it must be a rare case and that's one of reasons this bug didn't hit frequently.)
That being said, what we should fix is rather the function snd_dma_alloc_pages_fallback() to behave as expected, and it'll be like the patch below.
thanks,
Takashi
--- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -247,11 +247,10 @@ int snd_dma_alloc_pages_fallback(int type, struct device *device, size_t size, return err; if (size <= PAGE_SIZE) return -ENOMEM; + size >>= 1; aligned_size = PAGE_SIZE << get_order(size); if (size != aligned_size) size = aligned_size; - else - size >>= 1; } if (! dmab->area) return -ENOMEM;