Quoting Robin Murphy (2019-07-22 11:13:49)
Hi Chris,
On 20/07/2019 19:08, Chris Wilson wrote:
Since the cached32_node is allowed to be advanced above dma_32bit_pfn (to provide a shortcut into the limited range), we need to be careful to remove the to be freed node if it is the cached32_node.
Thanks for digging in - just to confirm my understanding, the problematic situation is where both 32-bit and 64-bit nodes have been allocated, the topmost 32-bit node is freed, then the lowest 64-bit node is freed, which leaves the pointer dangling because the second free fails the "free->pfn_hi < iovad->dma_32bit_pfn" check. Does that match your reasoning?
Yes. Once cached32_node is set to the right of dma_32bit_pfn it fails to be picked up in the next cycle through __cached_rbnode_delete_update should cached32_node be the next victim.
Assuming I haven't completely misread the problem, I can't think of a more appropriate way to fix it, so;
Reviewed-by: Robin Murphy robin.murphy@arm.com
I could swear I did consider that corner case at some point, but since Leizhen and I rewrote this stuff about 5 times between us I'm not entirely surprised such a subtlety could have got lost again along the way.
I admit to being impressed that rb_prev() does not appear high in the profiles -- though I guess that is more an artifact of the scary layers of caching above it. -Chris